Added possibility to allow trusted vendors to change code outside their namespace (#3156)

[0m3r]

Gives the possibility to add trusted vendors in package.json

"pwa-studio": {
    "targets": {
      "intercept": "./local-intercept.js"
    },
   "trusted-vendors":  [
      "@vendor"
   ]

and add custom checking into the file: packages/pwa-buildpack/lib/WebpackTools/ModuleTransformConfig.js _assertAllowedToTransform

Description

Added possibility to allow trusted vendors to change code outside their namespace

Related Issue

Closes #3156.

Acceptance

@sirugh
@zetlen

Verification Stakeholders

@jerschipper
@Jordaneisenburger

Verification Steps

in your project root package.json

"pwa-studio": {
    "targets": {
      "intercept": "./local-intercept.js"
    },
   "trusted-vendors":  [
      "@vendor"
   ]

in your extension intercept.js ('@vendor/extension/lib/intercept.js')

const { Targetables } = require('@magento/pwa-buildpack');
module.exports = targets => {
   const targetables = Targetables.using(targets);

   const HeaderComponent = targetables.reactComponent(
        '@magento/venia-ui/lib/components/Header/header.js'
    );
    
    HeaderComponent.insertAfterJSX(    
        '<SearchBar isOpen={isSearchOpen} ref={searchRef} />',
        `<span>INSERTED</span>`
    );
}

[PWAStudioBot]

	Messages
📖	DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖	Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Generated by 🚫 dangerJS against 1543a9c

[fooman]

@0m3r I like the direction where this is going. I am a little bit afraid that having this part of the .env makes a bit harder than necessary to administer though. Ideally the decision to add a vendor as trusted is done on a per project basis rather than on a per environment basis. It could lead to hard to track down issues once you end up with a list longer than a few vendors.

Ideally this configuration could live in a file that is part of the checked in code for the project. packages.json might a good fit as part of this:

  "pwa-studio": {
    "targets": {
      "intercept": "./local-intercept.js"
    }

to

  "pwa-studio": {
    "targets": {
      "intercept": "./local-intercept.js"
    },
   "trusted_vendors":  [
      "@vendor"
   ]

[larsroettig]

@0m3r I like the direction where this is going. I am a little bit afraid that having this part of the .env makes a bit harder than necessary to administer though. Ideally the decision to add a vendor as trusted is done on a per project basis rather than on a per environment basis. It could lead to hard to track down issues once you end up with a list longer than a few vendors.

Ideally this configuration could live in a file that is part of the checked in code for the project. packages.json might a good fit as part of this:

  "pwa-studio": {
    "targets": {
      "intercept": "./local-intercept.js"
    }

to

  "pwa-studio": {
    "targets": {
      "intercept": "./local-intercept.js"
    },
   "trusted_vendors":  [
      "@vendor"
   ]

Hi @0m3r. I totally agree with @fooman from my point of view this would be very nice if we can do this via package.json.
Currently, I will ask the core team about their opinion.

kind regards,

Lars

[0m3r]

Currently, I will ask the core team about their opinion.

what about core team opinion?

[larsroettig]

@supernova-at agreed with me on this yesterday would be cool if you can do it

[0m3r]

@larsroettig, @supernova-at review this commit e29c51e, please

[larsroettig]

Hi @0m3r,
The code looks good pls. Add test coverage, fix the two failing test cases and run yarn run prettier.

packages/peregrine/lib/targets/__tests__/peregrine-targets.spec.js
packages/venia-ui/lib/targets/__tests__/venia-ui-targets.spec.js

Thank you in advance. Also, I recommend joining as Slack then we answer questions mutch faster

https://devdocs.magento.com/community/resources.html

[0m3r]

Can I fix test fails in "my" code?

something like below

        const fs = require('fs');
...
        const context = path.resolve(__dirname, '../../../../../');
        const configPath = path.resolve(context, 'package.json');
        try {
            if (fs.existsSync(configPath)) {
                const config = require(configPath)['pwa-studio'];
                return config && config['trusted_vendors'] ? config['trusted_vendors'] : [];
                //file exists
            }
        } catch(err) {
          // console.error(err)
        }
        return [];

[larsroettig]

Hi @jimbo.
what do think about this PR like the idea and is very helpful for partners.

kind regards.,

Lars

[larsroettig]

One think about Extension package.json if some extension it allows itself via

trusted_vendors":  [
      "@vendor"
   ]

Should be prevented in every case a yarn install should be not enough to get access to this low level to react extension point for security reasons.

Verification Steps

• Checks for trusted_vendors will only be parsed from the root project package.json

@jimbo if we accept this from the core team we should ask also for adding test case to https://github.com/larsroettig/pwa-studio/blob/e5d4c42855627c4365fb3313f52837945a30aeb4/packages/pwa-buildpack/lib/WebpackTools/__tests__/MagentoResolver.spec.js

[jimbo]

Hi @jimbo.
what do think about this PR like the idea and is very helpful for partners.

I think it's great. We can certainly do more to expose components + API, as we do for talons, but this is a great way to relieve people's pain while we figure out those details.

[tjwiebell]

@0m3r Add support for monorepo and I think @larsroettig was also requesting test coverage if possible. Otherwise this looks perfect, nice work 👍

[0m3r]

@0m3r Add support for monorepo and I think @larsroettig was also requesting test coverage if possible. Otherwise this looks perfect, nice work

about tests

I did not find ModuleTransformConfig.spec.js
Unfortunately, I don't have enough experience to write this from scratch.

[tjwiebell]

@0m3r Existing code appears to be tested by integration tests in other packages, there is no unit test. Instead of building all that out in this scope, I've created a manual test that we'll run during regression and aim to automate if it becomes a priority. Thank you for the contribution.

✅ QA Passed

Added manual test for now