magento / pwa-studio

šŸ› Development tools to build, optimize and deploy Progressive Web Applications for Magento 2.
https://developer.adobe.com/commerce/pwa-studio/
Open Software License 3.0
1.07k stars 683 forks source link

[bug]: Extensions shouldn't have to tap `specialFeatures` constantly #2628

Closed zetlen closed 4 years ago

zetlen commented 4 years ago

Any extension with a React component, or any file that will go into the bundle, has to do this in its intercept: file

targets.of('@magento/pwa-buildpack').tap(features => {
  features[targets.name] = {
    esModules: true,
    cssModules: true
  }
});

This is tedious, because most extensions WILL contain those types of modules. These flags should be set by default for every module.

The specialFeatures target is meant for extensions to flag that they contain certain file types which will be included into the build. Without explicitly including extensions into Webpack rule patterns, they wouldn't be parsed as ES Modules or CSS modules, because Webpack doesn't do that for all node_modules by default. If we made it do that, we'd have extremely slow builds, so we do need to explicitly include extensions.

But we can make the Webpack JS and CSS loaders include all code inside registered extensions and assume it's CSS and JS modules. Then, a lot of that boilerplate can go away.

Possible solutions Implementation notes

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

m2-assistant[bot] commented 4 years ago

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


awilcoxa commented 4 years ago

Created PWA-866

larsroettig commented 4 years ago

@magento I am working on this

larsroettig commented 4 years ago

@zetlen runPhase already know about what are is active PWA Studio extensions.

https://github.com/magento/pwa-studio/blob/6da17c185871bbc1260646be04dfc7a1d6e76435/packages/pwa-buildpack/lib/BuildBus/BuildBus.js#L179-L202

tjwiebell commented 4 years ago

Addressed by #2765, going to close this out.