Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature]: Add the possibility to allow certain packages to change code outside their namespace #3156

Closed
2 of 8 tasks
Jordaneisenburger opened this issue May 3, 2021 · 7 comments · Fixed by #3266
Closed
2 of 8 tasks
Assignees
Labels
enhancement New feature or request Progress: done

Comments

@Jordaneisenburger
Copy link
Member

Is your feature request related to a problem? Please describe.
As an agency we sometimes need to change @magento/package code from within our own packages @experius/package as an example we need to change the way a part of pwa studio works or do a bugfix that's not yet in the current released version of PWA Studio.

A more concrete example would be this bugfix to the core. It's merged but wasn't shipped with 10.0. So I'd like to patch this file with a targetable for our projects from within our own package @experius/ui

Unfortunately this isn't allowed, we either need to have the same namspace @magento/peregrine or be inside the local project. This safety check is added for security reasons. But it's also limiting to agencies because we don't want to maintain default (meaning we need this for all our projects) targetables on 20 different projects inside the customers source.

Describe the solution you'd like
I'd love to see a way where the developer can explicitly tell webpack to also allow package x,y and z to change code from other packages. This way we are much more flexible but still keep the security as tight since the developer needs to manually allow a certain package in their local project's webpack.

Describe alternatives you've considered

Additional context

Please let us know what packages this feature is in regards to:

  • venia-concept
  • venia-ui
  • pwa-buildpack
  • peregrine
  • pwa-devdocs
  • upward-js
  • upward-spec
  • create-pwa
@Jordaneisenburger Jordaneisenburger added the enhancement New feature or request label May 3, 2021
@m2-assistant
Copy link

m2-assistant bot commented May 3, 2021

Hi @Jordaneisenburger. Thank you for your report.
To help us process this issue please make sure that you provided sufficient information.

Please, add a comment to assign the issue: @magento I am working on this


@sirugh
Copy link
Contributor

sirugh commented May 3, 2021

@magento export issue to JIRA project PWA as Story

@github-jira-sync-bot
Copy link

✅ Jira issue https://jira.corp.magento.com/browse/PWA-1702 is successfully created for this GitHub issue.

@jerschipper
Copy link

Is it possible for me to take a look into this and create a PR for this issue? I would also like to have this feature, but i'm not sure on how the core team thinks about this enhancement?

@sirugh
Copy link
Contributor

sirugh commented May 4, 2021

@jerschipper we welcome contributions! If you'd like to give it a shot, please do, and let us know that you are working on it.

@sirugh
Copy link
Contributor

sirugh commented May 27, 2021

@jerschipper are you still working on this?

@jerschipper
Copy link

@sirugh I really would like to do this. But i'm having trouble finding the correct time for it.
The idea i had to solve this was to make the
file: packages/pwa-buildpack/lib/WebpackTools/ModuleTransformConfig.js
and in that file the isBuiltin method to not to check on the package.json name, but make it check on the pwa-studio key in the package.json. That key is required to make the logic work and should be in my opinion enough to allow such changes.

How do you think about this modification @sirugh ?

tjwiebell added a commit that referenced this issue Sep 1, 2021
…ir namespace (#3156) (#3266)

* Added possibility to allow trusted vendors to change code outside their namespace (#3156)
* Read trusted vendors from root project package.json
* Checking if package.json exists (Wrong context in some tests)
* Rename trusted_vendors to trusted-vendors
* use process.cwd instead of hard context
* Run prettier

Co-authored-by: Lars Roettig <l.roettig@techdivision.com>
Co-authored-by: Tommy Wiebell <twiebell@adobe.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Progress: done
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants