Move the __non_webpack_import__ re-writing into the Babel plugin
#17637
Conversation
Snuffleupagus
force-pushed
the
babel-plugin-__non_webpack_import__
branch
2 times, most recently
from
c58a721 to
a15cc7f
Compare
There was a problem hiding this comment.
Looks good to me; I like the overall simplification here and that all rewriting now happens in a single Babel plugin. Let's await Nicolò's review before merging to make sure I haven't missed anything here or to see if there is an even better way to do this. Thanks!
@nicolo-ribaudo That could be a better idea, however I'm not entirely sure how to implement that. Also, thanks a lot for your help/feedback on these PRs! |
Note how we're using custom `__non_webpack_import__`-calls in the code-base, that we replace during the post-processing stage of the build, to be able to write `import`-calls that Webpack will leave alone during parsing. This work-around is necessary since we let Babel discards all comments, given that we generally don't need/want them in the builds, hence why we cannot utilize `/* webpackIgnore: true */`-comments in the source-code. After the changes in PR 17563 it thus seems to me that we should be able to just move this re-writing into the Babel plugin instead.
Snuffleupagus
force-pushed
the
babel-plugin-__non_webpack_import__
branch
from
a15cc7f to
4ab0ad3
Compare
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/d354cb37d3d987b/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/e22e7a68d4ed43d/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/e22e7a68d4ed43d/output.txt Total script time: 25.03 mins
Image differences available at: http://54.241.84.105:8877/e22e7a68d4ed43d/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/d354cb37d3d987b/output.txt Total script time: 39.72 mins
Image differences available at: http://54.193.163.58:8877/d354cb37d3d987b/reftest-analyzer.html#web=eq.log |
|
While it may be possible to improve this further, as outlined in #17637 (review), even as-is this patch is an improvement since it gets rid of manual post-processing and thus simplifies the build process. Landing with the previous r+, since the review comment above has been addressed. |
Note how we're using custom
__non_webpack_import__-calls in the code-base, that we replace during the post-processing stage of the build, to be able to writeimport-calls that Webpack will leave alone during parsing.This work-around is necessary since we let Babel discards all comments, given that we generally don't need/want them in the builds, hence why we cannot utilize
/* webpackIgnore: true */-comments in the source-code.After the changes in PR #17563 it thus seems to me that we should be able to just move this re-writing into the Babel plugin instead.
/cc @nicolo-ribaudo Since my knowledge of Babel is somewhat limited: Can you please check if this re-writing makes sense, or if there's a better/nicer way to implement this?