Scripts: Convert legacy entry point arguments for compatibility with webpack 5 #34264
Conversation
pablinos
changed the title
|
Size Change: 0 B Total Size: 1.04 MB ℹ️ View Unchanged
|
Soean
added
[Tool] WP Scripts
|
Fixes #34236 |
|
@pablinos, good investigation. I arrived at the same conclusion, see my comment #34236 (comment). |
Ah great! I missed that issue. Does that mean this is already fixed or handled in another PR? |
|
You are first, I plan to look at this PR tomorrow and test it inside Gutenberg. The way I do it looks as follows: npx wp-create-block example --no-wp-scripts
cd example
../node_modules/.bin/wp-scripts build src/index.jsThis way you are able to test the latest version of |
|
I did some tests and it looks like preserving weback v4 experience is going to be difficult. When I try to build my test block and run: ../node_modules/.bin/wp-scripts build src/index.jsI see the following error:
It might be similar to what we had in webpack 4 but it was handled by the custom code that gets removed in this PR. Then I tried: ../node_modules/.bin/wp-scripts build ./src/index.jsIt works fine but the output is different:
It looks like the default entry point is The other issue I found is when you provide multiple entry points they all get bundled into one file ../node_modules/.bin/wp-scripts build ./src/index.js ./src/view.jsOutput:
|
|
The good news is that I followed the comment #34239 (comment) from @walbo and I executed the following: ../node_modules/.bin/wp-scripts build --entry ./src/index.js --output-path=custom --output-filename=index.jsAnd it works like a charm for JavaScript:
It doesn't quite work with CSS 😅 |
|
Yes, this certainly looks a bit more complex than just removing the code. Thanks for the testing tips, I'll give it a go and see if I can get parity with the webpack v4 version. |
|
I did some research into how we might be able to fix this, and I'm not sure if this particularly elegant, but it's working for me. The principle is that we take the old syntax for specifying entry points on the command line and pass them to webpack as an environment variable. These environment variables can be passed as objects, so if the environment variable exists, then it gets used as the This also seems to fix #34239 for me as I can specify an entry point and a custom output path. @gziolo and @Soean - would you be able to give this a test and see if it's working for you? |
|
@pablinos, that's a great idea to tackle it this way. We probably can simplify it and use a hardcoded node ENV inside the script as we do with other options: |
|
Thanks @gziolo, I was thinking it could be simplified, but wasn't sure how. I don't think I fully understand your suggestion though! The complexity for me comes from the parsing and reformatting of the options. Would this help with that or is it solving another issue? Would we also want to use the same arguments with other scripts, like |
pablinos
changed the title
I can have a look tomorrow. If I don't come up with a better approach, we can proceed with your proposal. My biggest concern is that
This PR changes |
|
Thanks taking a look at this @gziolo
I'll test that some more, but that should work. The problem would be trying to override this
Yes, that's what I thought. I saw that the code you linked to was part of the If you think it would be better to use |
gziolo
force-pushed
the
fix/webpack-entrypoint-args
branch
from
b0ae1fb to
83b1807
Compare
|
@pablinos, I added a commit to better explain the idea I had: 83b1807 I think it doesn't cover the concern you had with I tested with the following command and it seems to work great: wp-scripts build src/index.js x=./src/view.js --output-path=customIt creates two entry points:
All paths passed are cleaned up from the CLI arguments to avoid some random side effects. Let me know what you think. I'm happy to mix both approach if you think that |
gziolo
added
the
Backwards Compatibility
My concern there was that if someone wanted to use the You're using the more obscurely named
That's a good addition, we should definitely keep that.
I personally feel like the |
I wasn't aware that
Ah right, a custom One more thing we need to add is to bail out of the legacy path handling when |
gziolo
changed the title
gziolo
changed the title
…webpack 5 (#34264) * Remove code to convert entrypoint argument conversion * Change entrypoint configuration to be passed by environment variables. * Change the operator to support older versions of node. * Change the logic to support = syntax for specifying output filenames. * Refactor the logic to use `process.env.WP_ENTRY` * Update webpack.config.js * Apply suggestions from code review * Update packages/scripts/config/webpack.config.js * Improve handling when `--entry` arg present Co-authored-by: Grzegorz Ziolkowski <[email protected]>
|
I cherry-picked this PR to Gutenberg 11.5 release branch. |
* trunk: [RNMobile][Embed block] Add device's locale to preview content (#33858) Update AlignmentMatrixControl docs post merge. (#34662) Chore: Update caniuse package to the latest version (#34685) Chore: Move `react-native-url-polyfill` to dev dependencies (#34687) Site Editor - add basic plugin support (#34460) ESLint Plugin: Use Jest related rules only when the package is installed (#33120) Update `@wordpress/components` package's contributing guidelines (#33960) chore(release): publish Update changelog files [RNMobile] [Embed block] Fix content disappearing on Android when switching light/dark mode (#34207) Scripts: Convert legacy entry point arguments for compatibility with webpack 5 (#34264) Update justication control in `flex` layout (#34651) Block Editor: Rename experimental prop used in `BlockControls` (#34644) Fix social links deprecation (#34639)
Description
Fixes #34236.
Fixes #34239.
After upgrading to v18.0.0, we started getting an error when specifying the entry point file on the command line:
Because
src/index.jsis the default, we could get around this problem by removing the command line argument, but I decided to investigate what was happening.It seems like the issue is caused by a breaking change from the upgrade to webpack v5. Looking at the documentation, webpack v4 states that custom/multiple entry points can be specified on the command line using
[name]=[file]pairs. The webpack v5 documentation doesn't have this option for command line entry points. One of the options it does show is simply the filenames listed as separate arguments.This change removes the code that converted file arguments on the command line to the webpack v4 entry point format, so the arguments are passed as is to webpack v5.
How has this been tested?
It's been a little tricky to test as I haven't been able to link the development version of the package with it's dependencies into our project in a way that will have the build process complete successfully. But I have been able to test that without this change, I get the above error, and with it, the error doesn't occur.
Types of changes
Bug fix - This allows entry points to be specified on the command line.
Checklist:
*.native.jsfiles for terms that need renaming or removal).