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

WIP (DO NOT MERGE) Migrate to Webpack 4 and webpack-serve #167

Closed
wants to merge 4 commits into from

Conversation

zetlen
Copy link
Contributor

@zetlen zetlen commented Jul 22, 2018

This PR is a:

[ ] New feature
[x] Enhancement/Optimization
[ ] Refactor
[ ] Bugfix
[ ] Test for existing code
[ ] Documentation

Summary

WIP (DO NOT MERGE): New dev server concept

Checking in this WIP (which is not presently working) because it
contains concepts we will want to bring forward, but implemented in a
way that is probably unsustainable.

In order:

  • Updated npm scripts so that common tasks were easier from package
    root
  • Began effort to switch from Magento rendered app shell to
    middle-tier-rendered app shell.
  • Began with HtmlWebpackPlugin and its extensions.
  • Found incompatibilities with webpack-dev-server in desired
    approach.
  • Began switch to webpack-serve over the unmaintained
    webpack-dev-server.
  • Began switch to Webpack 4 to maintain compatibility with
    webpack-serve. (Maybe you see where this is going.)
  • Switched from deprecated CommonsChunkPlugin to splitChunks
    configuration.
  • Tweaked splitChunks configuration and HtmlWebpackPlugin
    configuration to fix a topological sort issue caused by
    MagentoRootComponentsPlugin.
  • Refactored MagentoRootComponentsPlugin to be compatible with Webpack 4
    compilation API. (Thanks to @adrien-louis-r whose commits I
    cherry-picked, with attribution, to begin work on the plugin.)
  • Discovered root components are fundamentally incompatible with Webpack4
    with splitChunks in use.
  • Determined that the RootComponents manifest strategy might not be
    necessary at build time, if the middle tier specification can instead
    select entry points based on preload.
  • Decided to "park" this webpack-dev-mode upgrade effort for now.

Additional information

Opened this as a PR so that it's easy to comment on and visible to
collaborators and community, but please do not merge this. It would be
very bad to merge this.

zetlen and others added 4 commits July 22, 2018 15:25
WIP about to merge webpack-serve

WIP switch to webpack-serve

WIP bugfixing webpack-serve branch
Signed-off-by: James Zetlen <[email protected]>
Checking in this WIP (which is not presently working) because it
contains concepts we will want to bring forward, but implemented in a
way that is probably unsustainable.

In order:
- Updated npm scripts so that common tasks were easier from package
 root
- Began effort to switch from Magento rendered app shell to
 middle-tier-rendered app shell.
- Began with HtmlWebpackPlugin and its extensions.
- Found incompatibilities with `webpack-dev-server` in desired
  approach.
- Began switch to `webpack-serve` over the unmaintained
  `webpack-dev-server`.
- Began switch to Webpack 4 to maintain compatibility with
`webpack-serve`. _(Maybe you see where this is going.)_
- Switched from deprecated CommonsChunkPlugin to `splitChunks`
configuration.
- Tweaked `splitChunks` configuration and HtmlWebpackPlugin
configuration to fix a topological sort issue caused by
MagentoRootComponentsPlugin.
- Refactored MagentoRootComponentsPlugin to be compatible with Webpack 4
compilation API. (Thanks to @adrien-louis-r whose commits I
cherry-picked, with attribution, to begin work on the plugin.)
- Discovered root components are fundamentally incompatible with Webpack4
with splitChunks in use.
- Determined that the RootComponents manifest strategy might not be
necessary at build time, if the middle tier specification can instead
select entry points based on preload.
- Decided to "park" this webpack-dev-mode upgrade effort for now.

Opened this as a PR so that it's easy to comment on and visible to
collaborators and community, but please do not merge this. It would be
very bad to merge this.
@zetlen zetlen changed the title Zetlen/buildpack webpack serve migrate Migrate to Webpack 4 and webpack-serve Jul 22, 2018
@zetlen zetlen changed the title Migrate to Webpack 4 and webpack-serve WIP (DO NOT MERGE) Migrate to Webpack 4 and webpack-serve Jul 22, 2018
@PWAStudioBot
Copy link
Contributor

Fails
🚫

The following file(s) were not formatted with prettier. Make sure to execute npm run prettier locally prior to committing.

packages/pwa-buildpack/src/WebpackTools/__tests__/PWADevServer.spec.js
packages/pwa-buildpack/src/WebpackTools/plugins/__tests__/DevServerReadyNotifierPlugin.spec.js
packages/pwa-buildpack/src/WebpackTools/plugins/DevServerReadyNotifierPlugin.js
🚫

The following unit tests did not pass 😔. All tests must pass before this PR can be merged

packages/pwa-buildpack/src/WebpackTools/plugins/__tests__/ServiceWorkerPlugin.spec.js
  ● throws if options are missing
expect(function).toThrow(string)

Expected the function to throw an error matching:
  "env.phase must be of type string"
Instead, it threw:
  BuildpackValidationError: ServiceWorkerPlugin: Invalid configuration object. ServiceWorkerPlugin was called with a configuration object that has the following problems:
	- env.mode must be of type string
	- serviceWorkerFileName must be of type string
      39 |     );
      40 |     if (invalid.length > 0) {
    > 41 |         throw new BuildpackValidationError(name, callsite, invalid);
         |               ^
      42 |     }
      43 | };
      44 | 

  at Function.validateOptions (src/util/options-validator.js:41:15)
  at new ServiceWorkerPlugin (src/WebpackTools/plugins/ServiceWorkerPlugin.js:11:29)
  at expect (src/WebpackTools/plugins/__tests__/ServiceWorkerPlugin.spec.js:10:18)
  at Object.<anonymous> (../../node_modules/expect/build/to_throw_matchers.js:51:9)
  at Object.throwingMatcher [as toThrow] (../../node_modules/expect/build/index.js:320:33)
  at Object.<anonymous>.test (src/WebpackTools/plugins/__tests__/ServiceWorkerPlugin.spec.js:10:47)
  at Object.<anonymous>.test (src/WebpackTools/plugins/__tests__/ServiceWorkerPlugin.spec.js:10:47)

● returns a valid Webpack plugin

BuildpackValidationError: ServiceWorkerPlugin: Invalid configuration object. ServiceWorkerPlugin was called with a configuration object that has the following problems:
	- env.mode must be of type string

  39 |     );
  40 |     if (invalid.length > 0) {
> 41 |         throw new BuildpackValidationError(name, callsite, invalid);
     |               ^
  42 |     }
  43 | };
  44 | 

  at Function.validateOptions (src/util/options-validator.js:41:15)
  at new ServiceWorkerPlugin (src/WebpackTools/plugins/ServiceWorkerPlugin.js:11:29)
  at Object.<anonymous>.test (src/WebpackTools/plugins/__tests__/ServiceWorkerPlugin.spec.js:27:9)

● .apply calls WorkboxPlugin.GenerateSW in prod

BuildpackValidationError: ServiceWorkerPlugin: Invalid configuration object. ServiceWorkerPlugin was called with a configuration object that has the following problems:
	- env.mode must be of type string

  39 |     );
  40 |     if (invalid.length > 0) {
> 41 |         throw new BuildpackValidationError(name, callsite, invalid);
     |               ^
  42 |     }
  43 | };
  44 | 

  at Function.validateOptions (src/util/options-validator.js:41:15)
  at new ServiceWorkerPlugin (src/WebpackTools/plugins/ServiceWorkerPlugin.js:11:29)
  at Object.<anonymous>.test (src/WebpackTools/plugins/__tests__/ServiceWorkerPlugin.spec.js:41:20)

● .apply calls nothing but warns in console in dev

BuildpackValidationError: ServiceWorkerPlugin: Invalid configuration object. ServiceWorkerPlugin was called with a configuration object that has the following problems:
	- env.mode must be of type string

  39 |     );
  40 |     if (invalid.length > 0) {
> 41 |         throw new BuildpackValidationError(name, callsite, invalid);
     |               ^
  42 |     }
  43 | };
  44 | 

  at Function.validateOptions (src/util/options-validator.js:41:15)
  at new ServiceWorkerPlugin (src/WebpackTools/plugins/ServiceWorkerPlugin.js:11:29)
  at Object.<anonymous>.test (src/WebpackTools/plugins/__tests__/ServiceWorkerPlugin.spec.js:71:20)

● .apply generates and writes out a serviceworker when enableServiceWorkerDebugging is set

BuildpackValidationError: ServiceWorkerPlugin: Invalid configuration object. ServiceWorkerPlugin was called with a configuration object that has the following problems:
	- env.mode must be of type string

  39 |     );
  40 |     if (invalid.length > 0) {
> 41 |         throw new BuildpackValidationError(name, callsite, invalid);
     |               ^
  42 |     }
  43 | };
  44 | 

  at Function.validateOptions (src/util/options-validator.js:41:15)
  at new ServiceWorkerPlugin (src/WebpackTools/plugins/ServiceWorkerPlugin.js:11:29)
  at Object.<anonymous>.test (src/WebpackTools/plugins/__tests__/ServiceWorkerPlugin.spec.js:98:20)
packages/pwa-buildpack/src/WebpackTools/__tests__/PWADevServer.spec.js
  ● .configure() throws errors on missing config
expect(received).rejects.toThrow()

Expected received Promise to reject, instead it resolved to value
  {"add": [Function add], "clipboard": false, "content": [undefined], "hotClient": {"https": true}}

  189 | 
  190 | test('.configure() throws errors on missing config', async () => {
> 191 |     await expect(
      |           ^
  192 |         PWADevServer.configure({
  193 |             id: 'foo',
  194 |             publicPath: 'bar',

  at expect (../../node_modules/expect/build/index.js:100:15)
  at Object.<anonymous>.test (src/WebpackTools/__tests__/PWADevServer.spec.js:191:11)

● .configure() returns a configuration object for the serve property of a webpack config

expect(value).toMatchSnapshot()

Received value does not match stored snapshot ".configure() returns a configuration object for the `serve` property of a webpack config 1".

- Snapshot
+ Received

  Object {
    "add": [Function],
    "clipboard": false,
    "content": Array [
-     "path/to/assets",
+     undefined,
    ],
    "hotClient": Object {
-     "logLevel": "warning",
-     "stats": "minimal",
+     "https": true,
    },
  }

  269 |     const devServer = await PWADevServer.configure(config);
  270 | 
> 271 |     expect(devServer).toMatchSnapshot();
      |                       ^
  272 | });
  273 | 
  274 | test('.configure() returns a configuration object with before() and after() handlers that add middlewares in order', async () => {

  at Object.<anonymous>.test (src/WebpackTools/__tests__/PWADevServer.spec.js:271:23)

● .configure() returns a configuration object with before() and after() handlers that add middlewares in order

expect(jest.fn()).toHaveBeenCalled()

Expected mock function to have been called, but it was not called.

  298 |     };
  299 |     devServer.add(app);
> 300 |     expect(app.use).toHaveBeenCalled();
      |                     ^
  301 | });
  302 | 
  303 | 

  at Object.<anonymous>.test (src/WebpackTools/__tests__/PWADevServer.spec.js:300:21)

Generated by 🚫 dangerJS

@zetlen
Copy link
Contributor Author

zetlen commented Sep 18, 2018

No longer relevant due to #248 and oncoming loadable.

@zetlen zetlen closed this Sep 18, 2018
@zetlen zetlen deleted the zetlen/buildpack-webpack-serve-migrate branch November 9, 2018 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants