-
Notifications
You must be signed in to change notification settings - Fork 12
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
implement webpack chunks file updating using ast manipulation #12
Conversation
fe74186
to
7d5af2d
Compare
7d5af2d
to
a06218f
Compare
That's amazing @dario-piotrowicz Could you please update I need to go for 1h, I'll review after that. Thanks! |
Thanks ❤️
I've removed the section regarding updating the config file entirely as I think that that is now totally unnecessary and that we should not bother telling users to set the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial notes on all but the actual transform files 😄
...uild/patches/investigated/update-webpack-chunks-file/test-fixtures/minified-webpacks-file.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic work @dario-piotrowicz
builder/src/build/patches/investigated/update-webpack-chunks-file/index.ts
Outdated
Show resolved
Hide resolved
.../build/patches/investigated/update-webpack-chunks-file/get-chunk-installation-identifiers.ts
Outdated
Show resolved
Hide resolved
.../build/patches/investigated/update-webpack-chunks-file/get-chunk-installation-identifiers.ts
Show resolved
Hide resolved
...d/patches/investigated/update-webpack-chunks-file/get-updated-webpack-chunks-file-content.ts
Outdated
Show resolved
Hide resolved
...estigated/update-webpack-chunks-file/get-file-content-with-updated-webpack-require-f-code.ts
Outdated
Show resolved
Hide resolved
...estigated/update-webpack-chunks-file/get-file-content-with-updated-webpack-require-f-code.ts
Outdated
Show resolved
Hide resolved
...estigated/update-webpack-chunks-file/get-file-content-with-updated-webpack-require-f-code.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's awesome.
I added a few minor comments.
...d/patches/investigated/update-webpack-chunks-file/get-chunk-installation-identifiers.test.ts
Outdated
Show resolved
Hide resolved
.../build/patches/investigated/update-webpack-chunks-file/get-chunk-installation-identifiers.ts
Show resolved
Hide resolved
.../build/patches/investigated/update-webpack-chunks-file/get-chunk-installation-identifiers.ts
Outdated
Show resolved
Hide resolved
.../build/patches/investigated/update-webpack-chunks-file/get-chunk-installation-identifiers.ts
Show resolved
Hide resolved
builder/src/build/patches/investigated/update-webpack-chunks-file/index.ts
Show resolved
Hide resolved
builder/src/build/patches/investigated/update-webpack-chunks-file/index.ts
Outdated
Show resolved
Hide resolved
...uild/patches/investigated/update-webpack-chunks-file/test-fixtures/minified-webpacks-file.js
Outdated
Show resolved
Hide resolved
65d30a6
to
d009803
Compare
this allows us not to have to require the use of the `serverMinification: false` option (as we're no longer relying on known variable names)
Co-authored-by: Pete Bacon Darwin <pbacondarwin@cloudflare.com>
d009803
to
5272a2a
Compare
add ts-doc comments to newly introduced functions
generalize `getWEbpackChunksFileTsSource` to `tsParseFile`
update webpackFRequireFunction detection not to use unclear `find(Boolean)`
this allows us not to have to require the use of the
serverMinification: false
option(as we're no longer relying on known variable names)