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

Custom Template Path Webpack Plugin: Rewrite as CommonJS #15709

Merged
merged 2 commits into from
May 20, 2019

Conversation

aduth
Copy link
Member

@aduth aduth commented May 17, 2019

Extracted from: #15226

This pull request seeks to expose the source of CustomTemplatedPathWebpackPlugin directly as its source, omitting it from the Babel transpilation build step. The proposed benefit is to reduce overhead by limiting the number of Babel-transpiled packages, to accurately reflect the current source as being CommonJS and, depending on the outcome of #15226, avoid dependencies on a built package from the project's own build tooling (Webpack). The main downside is fragmentation on which packages are and aren't transpiled by Babel.

Testing instructions:

Verify build succeeds:

npm run build

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Same questions as #15710

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

How about the lock file? It should get updated as well.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

My comment from the second similar PR:

See: https://medium.com/@nodejs/announcing-a-new-experimental-modules-1be8d2d6c2ff

Add “type”: “module” to the package.json for your project, and Node.js will treat all .js files in your project as ES modules.

There is also commonjs value proposed.

We should add this new field to all packages and start using it with the script which picks which packages should be transpiled. This is going to solve the issue with special handling for src folder.

@aduth aduth force-pushed the update/webpack-plugins-common-js branch from 78008f1 to 9f8f063 Compare May 20, 2019 13:47
@aduth
Copy link
Member Author

aduth commented May 20, 2019

Resolved package-lock.json in rebased 9f8f063.

@aduth
Copy link
Member Author

aduth commented May 20, 2019

We should add this new field to all packages and start using it with the script which picks which packages should be transpiled. This is going to solve the issue with special handling for src folder.

Do we have a related issue for this? It has some other impact on how Node treats code that we might want to be considerate of (e.g. safe to direct build-module/index.js as main?).

@aduth aduth merged commit 3e14a30 into master May 20, 2019
@aduth aduth deleted the update/webpack-plugins-common-js branch May 20, 2019 15:27
@gziolo
Copy link
Member

gziolo commented May 20, 2019

We should add this new field to all packages and start using it with the script which picks which packages should be transpiled. This is going to solve the issue with special handling for src folder.

Do we have a related issue for this? It has some other impact on how Node treats code that we might want to be considerate of (e.g. safe to direct build-module/index.js as main?).

I haven’t created one yet. ES modules in Node 12 are still behind the flag. You can’t safely use them in CLI scripts in our setup. However we can add this new field for future usage and to better describe the type of package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants