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

[bug]: Difficulty upgrading mono-repo to 7.x because of pwa-buildpack targets #2717

Closed
2 of 8 tasks
brendanfalkowski opened this issue Sep 19, 2020 · 4 comments
Closed
2 of 8 tasks
Labels
bug Something isn't working Progress: done

Comments

@brendanfalkowski
Copy link
Contributor

Describe the bug
I am upgrading a mono-repo project that originated in 2018-11 from version 5.x to 7.x (diff taken 2020-07-22).

By mono-repo project, what I mean is we forked the full magento/pwa-studio and established the folder packages/custom which duplicated Venia UI/Concept and has been heavily customized. We have backmerged updates at 3.x, 4.x, 5.x, and now 7.x to maintain forward compatibility.

It's not clear how the targets/intercepts concepts can be lifted from packages/venia-ui and transplanted in packages/custom. The tooling in packages/pwa-buildpack is hardcoded to believe the build tooling always extends venia-ui but we don't use components of venia-ui in our frontend.

All diffs have been integrated and issues resolved by tracing failures from yarn run build (which targets our packages/custom not Venia). The build errors out on RichContent/richContentRenderers.js which attempts the reference the generated renderers in venia-ui.

ERROR in ../venia-ui/lib/components/RichContent/richContentRenderers.js
Module not found: Error: Can't resolve 'undefined' in '/Users/brendan/work/projectName/packages/venia-ui/lib/components/RichContent'
 @ ../venia-ui/lib/components/RichContent/richContentRenderers.js 6:0-41 8:16-27
 @ ../venia-ui/lib/components/RichContent/richContent.js
 @ ../venia-ui/lib/components/RichContent/index.js
 @ ../venia-ui/lib/RootComponents/Category/categoryContent.js
 @ ../venia-ui/lib/RootComponents/Category/category.js
 @ ../venia-ui/lib/RootComponents/Category/index.js
 @ ../pwa-buildpack/node_modules/webpack-inject-plugin/dist/webpack-inject-plugin.loader.js?id=webpack-inject-module-1
 @ multi ../pwa-buildpack/node_modules/webpack-inject-plugin/dist/webpack-inject-plugin.loader?id=webpack-inject-module-1 ./src/index.js

I tried various core edits to force the code in packages/pwa-buildpack and packages/venia-ui to stop using relative paths directed at the venia-ui package, but can't find the right combination that resolves the error. In fact, each edit throws more errors/warnings than the singular RichContent error (above).

I also explored eliminating targets/intercepts and RichContent from our custom package's build, but realized this isn't possible because PageBuilder requires this integration to function. PageBuilder is the #1 reason we need the 7.x updates (not module UI).

Additional context
I read the dev docs for PageBuilder and PWA Buildpack for background and relevant code, but don't understand how to make the build step that generates richContentRenderers.js work by configuration or core hacks. It's not clear if a mono-repo that doesn't run its build scripts off Venia packages can be made to work with the extensibility tooling that was created for "create-pwa" originated projects.

Our aim has been to avoid core changes to building tooling, and not worry about syncing between our components and Venia's components (the whole site's UI has already been built out). Ultimately we expect ever needing to extend Venia components or utilize third-party extensions for UI. We're only blocked by the PageBuilder implementation requiring the extensibility framework.

Please complete the following device information:

  • Magento Version: 2.3.5
  • PWA Studio Version: 7.x
  • NPM version npm -v: 6.9.0
  • Node Version node -v: 10.16.2

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

  • venia-concept
  • venia-ui
  • pwa-buildpack
  • peregrine
  • pwa-devdocs
  • upward-js
  • upward-spec
  • create-pwa
@brendanfalkowski brendanfalkowski added the bug Something isn't working label Sep 19, 2020
@m2-assistant
Copy link

m2-assistant bot commented Sep 19, 2020

Hi @brendanfalkowski. 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


@Jordaneisenburger
Copy link
Member

Hi @brendanfalkowski ,

I've build a mono-repo as well, we have a storefront scaffolded with the pwa studio storefront-concept as it's base structure. We've also created our own packages/ui and managed to get it working properly.

inside packages/ui/lib/targets/venia-ui-intercept.js

const isRCR = mod =>
    mod.resource ===
    path.resolve(
        __dirname,
        '@magento/venia-ui/lib/components/RichContent/richContentRenderers.js',
    );

const name = '@experius-pwa/ui';

// Guard condition for whether the loader has already been installed.
const loaderIsInstalled = mod => mod.loaders.some(l => l.ident === ident);

builtins.webpackCompiler.tap(compiler =>
    compiler.hooks.compilation.tap(name, compilation =>
        compilation.hooks.normalModuleLoader.tap(
            name,
            (loaderContext, mod) => {
                if (isRCR(mod) && !loaderIsInstalled(mod)) {
                    const renderers = [];
                    const api = {
                        add(renderer) {
                            renderers.push(renderer);
                        },
                    };
                    targets.own.richContentRenderers.call(api);
                    mod.loaders.push({
                        ident: ident,
                        loader,
                        options: {
                            renderers: renderers.reverse(),
                        },
                    });
                }
            },
        ),
    ),
);

// Dogfood our own richContentRenderer hook to insert the fallback renderer.
targets.own.richContentRenderers.tap(api =>
    api.add({
        componentName: 'Wysiwyg',
        importPath:
            '@experius-pwa/ui/lib/components/base/wysiwyg/wysiwyg.js',
    }),
);

Not sure if this helps, if you'd like we could jump on a quick call. let me know

@sirugh
Copy link
Contributor

sirugh commented Sep 30, 2020

Made https://jira.corp.magento.com/browse/PWA-955 for internal tracking.

@brendanfalkowski
Copy link
Contributor Author

brendanfalkowski commented Oct 5, 2020

Going to self-close this. The Magento team helped track this down. Details for those interested:

At the time we cloned PWA Studio to diff for upstream merging (2019-08-22), there was a mismatch (in "develop" branch) between the PageBuilder dependency (2.0.0 required) and the code in PWA Studio which used extensibility features to implement it. If the associated changes had been merged back to "develop" just before we pulled, then it would have worked. Just bad luck timing. So some exports were absent from the PageBuilder version we were targeting based on the upstream diff. Manually bumping PageBuilder fixed this without beginning another upstream merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Progress: done
Projects
None yet
Development

No branches or pull requests

3 participants