-
Notifications
You must be signed in to change notification settings - Fork 173
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
Migrate client-admin from gulp to webpack #1242
Merged
Merged
Changes from 35 commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
50c7af9
First-pass gulp-to-webpack conversion.
patcon 3ff0b6c
Update webpack-cli, and create prod/dev logic.
patcon 7b0dd8f
Messy: leave uncompressed files in dev mode build.
patcon 916a24d
Shut off compression in dev mode more elegantly.
patcon bf25149
Get docker build working.
patcon bc48153
Fixed bundlewatch path to find client-admin files.
patcon 562b9cf
Get rid of old webpack build scripts.
patcon 5f03c3b
Don't minify for dev build.
patcon 246f313
Removed dep pkgs for s3 deploy.
patcon eb02c29
Added ability to use bundle analyzer plugin.
patcon 7e4c587
Removed all unneeded files.
patcon 3ce8dec
Fixed up eslint issue with upgraded packages.
patcon c04184d
Fixed bundlewatch CI script.
patcon 7370aef
Keep new client-admin bundle consistently named.
patcon 46722d0
Try running e2e tests against sslip.io instead of localhost.
patcon a4e1052
Escape the domainWhitelist strings as it was done before.
patcon a413d67
Revert "Try running e2e tests against sslip.io instead of localhost."
patcon af5c73f
Oops. Inversed the boolean for configs.
patcon 9b95b8d
Trying to debug index_admin.html issue.
patcon f16b162
Added upterm for remote debugging.
patcon 3f3bc1d
Ensure upterm runs on failures.
patcon 343f9f0
Added publicPath to make script link absolute.
patcon ace4713
Drop the embed.html test file into new home.
patcon c443388
Ensuring dev mode builds and dev server use same simplifications.
patcon bc8f68c
Set up all routes to point to index.html. Added mention of future proxy.
patcon 0f8e9f8
Removed debugging step from github actions.
patcon 96f1234
Tiny fixups to clean up PR.
patcon dd4f5fc
Only write headerJson files during prod build.
patcon 14ee30a
Migrate from dist to more build as before.
patcon 40168f8
Removed unneeded config vars.
patcon 598c784
Cleaned up webpack dev-mode logic.
patcon 15ab437
Went back to using dist folder to fix bundle-analyzer bug.
patcon ff79830
Updated client-admin README. Removed badges. Removed deployment refer…
patcon b86dea6
Cleaned up ignorefiles.
patcon 2b03976
Mention where builds happen.
patcon 86b7031
resolve merge conflicts with edge
metasoarous 1421055
Use build instead of dist for client-admin build output
metasoarous a001045
switch to npm run build:prod in file-server/Dockerfile
metasoarous 7065bb1
inject compiled js in html body for client-admin
metasoarous 906d38f
add/update webpack versions
metasoarous File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
Empty file.
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Is
xxxxxxxx
a placeholder for something else?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.
@patcon Do you remember what the intention here was? Or was this just stubbed out and not revisited?
I know that in some places we timestamp file paths for caching, so I'm not sure if this was intended to fit into that scheme, or if it's something that didn't get finished.
Thanks
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.
Ack, sorry to leave this hanging! I was thinking the xxxxx was permanent solution.
It was because the bundlesize tool depends on files having the same filename between runs. So this just removes to the dynamic hash. This helps compare them and offer helpful feedback about changes:
console log with link: https://github.com/compdemocracy/polis/actions/runs/3079836348/jobs/4976514743#step:9:17
link to page
The link to the above page is only visible in CI logs when creating PRs from repo forks. But when PRs are created off branches in the upstream repo instead of forks, it gets a more clear link the the "checks" section of the PR.
tl;dr - without the xxxxx change, the bundlewatch tool can't know the change in size
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.
Thanks for getting back to us on this @patcon.
Am I correct in understanding that that this only affects the bundlewatch workflow, and not the production builds? I see the value of being able to present diffs with bundlewatch, but not at the expense of our current caching strategy.
One way or the other, we should have this documented in comments to avoid confusion in the future.
Thanks again
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.
no prob! but yes, this only happens in the bash line of the workflow, and nowhere else. so no effect on prod builds :)
This should be confirmed with
git grep .xxxxxxxx.js
and that should be the only place it happensThere 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.
OK; Great. Thank you for confirming. I'd appreciate a line comment about this so future devs know why it's happening, but otherwise looks good.