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

Use module property from dev webpack config in DLL webpack config #1497

Merged

Conversation

nessup
Copy link

@nessup nessup commented Apr 5, 2018

Closes #1468

To avoid a circular dependency, this relies on a check against the parent module's filename for webpack.config.renderer.dev.dll.js. It's expected that developers who rename config files will hopefully grep the codebase for this filename before changing it.

This change also includes this check again when configuring DllReferencePlugin, because it is no longer guaranteed that the DLL manifest file exists by the time the renderer config specifies plugins.

I played around with the idea of creating a webpack-merge strategy that plucks the value of module from the dev webpack config, but it wouldn't help much because, in webpack.config.renderer.dev.js, the DllReferencePlugin would still try to require the manifest (which would not exist).

Lmk if this should be rebased off a different tag.

BuckyMaler and others added 5 commits February 2, 2018 21:09
* Update webpack.config.renderer.dev.dll.js

* update webpack to support sass files

* update webpack to support sass files
Fixes electron-react-boilerplate#1468

To avoid a circular dependency, this relies on a check against the parent module's filename for `webpack.config.renderer.dev.dll.js`. It's expected that developers who rename config files will hopefully grep the codebase for this filename before changing it.

This change also includes this check again when configuring `DllReferencePlugin`, because it is no longer guaranteed that the DLL manifest file exists by the time the renderer config specifies plugins.

I played around with the idea of creating a `webpack-merge` strategy that plucks the value of `module` from the dev webpack config, but it wouldn't help much because, in `webpack.config.renderer.dev.js`, the `DllReferencePlugin` would still try to require the manifest (which would not exist).
@amilajack amilajack changed the base branch from master to v0.13.3 April 13, 2018 19:36
@amilajack
Copy link
Member

I changed the base of this PR to the v0.13.3. PR's should be made against this branch instead of master. Can you please fix the merge conflicts.

@nessup
Copy link
Author

nessup commented May 2, 2018

Conflicts resolved

@@ -23,11 +23,13 @@ const port = process.env.PORT || 1212;
const publicPath = `http://localhost:${port}/dist`;
const dll = path.resolve(process.cwd(), 'dll');
const manifest = path.resolve(dll, 'renderer.json');
const requiredByDLLConfig =
module.parent.filename.indexOf('webpack.config.renderer.dev.dll') !== -1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer ES6 semantics instead (.includes())

@amilajack amilajack merged commit c291fc8 into electron-react-boilerplate:v0.13.3 May 2, 2018
@amilajack
Copy link
Member

Thanks for this!

amilajack added a commit that referenced this pull request May 24, 2018
* Misc code style changes to menu.js

* v0.13.3

* More consistent node path

* Allowed node_modules to be checked by flow

* add prettier to format js files (#1418)

* Remove jsdom dep (#1411)

* Remove dynamic import dep (#1408)

* Add .sass files support (#1412)

* Update webpack.config.renderer.dev.dll.js

* update webpack to support sass files

* update webpack to support sass files

* Misc code style changes to menu.js

* chore: add perttier husky lint-stage eslint-config-prettier and add scripts

* refactor: use prettier to format code

* fix: fix lint error and add eslint prettier config

* fix: replace registry url from registry.npmjs.org to registry.yarnpkg.com for new add package

* docs: update changelog

* chore: add format-fix script and make format script just to check which files need to format

* format: use prettier to format webpack files

* docs: update change log - add format-fix script

* feat: add prettier in `lint-fix` script

* Removed unnecessary deps

* Updated deps

* Upgraded to webpack 4

* Run prettier even if eslint fails

* createBrowserHistory to createHashHistory for prod (#1184)

* Filter deps without entrypoint from dll

* Bumped deps

* Temporary hack to get flow working with webpack-cli

* Use module property from dev webpack config in DLL webpack config (#1497)

* Remove jsdom dep (#1411)

* Remove dynamic import dep (#1408)

* Add .sass files support (#1412)

* Update webpack.config.renderer.dev.dll.js

* update webpack to support sass files

* update webpack to support sass files

* Misc code style changes to menu.js

* Use module property from dev webpack config in DLL webpack config

Fixes #1468

To avoid a circular dependency, this relies on a check against the parent module's filename for `webpack.config.renderer.dev.dll.js`. It's expected that developers who rename config files will hopefully grep the codebase for this filename before changing it.

This change also includes this check again when configuring `DllReferencePlugin`, because it is no longer guaranteed that the DLL manifest file exists by the time the renderer config specifies plugins.

I played around with the idea of creating a `webpack-merge` strategy that plucks the value of `module` from the dev webpack config, but it wouldn't help much because, in `webpack.config.renderer.dev.js`, the `DllReferencePlugin` would still try to require the manifest (which would not exist).

* Use includes() rather than indexOf()

* Updated all deps to latest semver

* Bumped deps

* Bumped all deps to latest semver

* Update changelog

* Increased delay for e2e counter test

* Updated lock file

* Bumped ci node versions

* Reverted version change in CHANGELOG [ci skip]
vikr01 pushed a commit to vikr01/electron-react-boilerplate that referenced this pull request Jun 27, 2018
* Misc code style changes to menu.js

* v0.13.3

* More consistent node path

* Allowed node_modules to be checked by flow

* add prettier to format js files (electron-react-boilerplate#1418)

* Remove jsdom dep (electron-react-boilerplate#1411)

* Remove dynamic import dep (electron-react-boilerplate#1408)

* Add .sass files support (electron-react-boilerplate#1412)

* Update webpack.config.renderer.dev.dll.js

* update webpack to support sass files

* update webpack to support sass files

* Misc code style changes to menu.js

* chore: add perttier husky lint-stage eslint-config-prettier and add scripts

* refactor: use prettier to format code

* fix: fix lint error and add eslint prettier config

* fix: replace registry url from registry.npmjs.org to registry.yarnpkg.com for new add package

* docs: update changelog

* chore: add format-fix script and make format script just to check which files need to format

* format: use prettier to format webpack files

* docs: update change log - add format-fix script

* feat: add prettier in `lint-fix` script

* Removed unnecessary deps

* Updated deps

* Upgraded to webpack 4

* Run prettier even if eslint fails

* createBrowserHistory to createHashHistory for prod (electron-react-boilerplate#1184)

* Filter deps without entrypoint from dll

* Bumped deps

* Temporary hack to get flow working with webpack-cli

* Use module property from dev webpack config in DLL webpack config (electron-react-boilerplate#1497)

* Remove jsdom dep (electron-react-boilerplate#1411)

* Remove dynamic import dep (electron-react-boilerplate#1408)

* Add .sass files support (electron-react-boilerplate#1412)

* Update webpack.config.renderer.dev.dll.js

* update webpack to support sass files

* update webpack to support sass files

* Misc code style changes to menu.js

* Use module property from dev webpack config in DLL webpack config

Fixes electron-react-boilerplate#1468

To avoid a circular dependency, this relies on a check against the parent module's filename for `webpack.config.renderer.dev.dll.js`. It's expected that developers who rename config files will hopefully grep the codebase for this filename before changing it.

This change also includes this check again when configuring `DllReferencePlugin`, because it is no longer guaranteed that the DLL manifest file exists by the time the renderer config specifies plugins.

I played around with the idea of creating a `webpack-merge` strategy that plucks the value of `module` from the dev webpack config, but it wouldn't help much because, in `webpack.config.renderer.dev.js`, the `DllReferencePlugin` would still try to require the manifest (which would not exist).

* Use includes() rather than indexOf()

* Updated all deps to latest semver

* Bumped deps

* Bumped all deps to latest semver

* Update changelog

* Increased delay for e2e counter test

* Updated lock file

* Bumped ci node versions

* Reverted version change in CHANGELOG [ci skip]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants