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

Update JS dependencies and webpack #14118

Merged
merged 6 commits into from
Dec 27, 2020
Merged

Update JS dependencies and webpack #14118

merged 6 commits into from
Dec 27, 2020

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Dec 22, 2020

  • Update all JS dependencies
  • Adapt webpack config for version 5
  • Update to Less 4.0, adapting usage of removed mixin syntax
  • Enable new ESLint rules and fix discovered issues

@silverwind silverwind added this to the 1.14.0 milestone Dec 22, 2020
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Dec 23, 2020
@CirnoT
Copy link
Contributor

CirnoT commented Dec 23, 2020

webpack-cli can be updated to v4, it's backwards compatible

@silverwind
Copy link
Member Author

webpack-cli can be updated to v4, it's backwards compatible

cli flags seem to have changed in an incompatible way and it also seems compilation gets slower with the upgraded cli. I'll check thought.

[webpack-cli] Unknown argument: --hide-modules
[webpack-cli] Unknown argument: --display-entrypoints=false

@silverwind
Copy link
Member Author

silverwind commented Dec 23, 2020

I don't think we can upgrade webpack-cli without also upgrading webpack because of those output options. It seems the new way to set them is in the stats option in the config but they seem to have no effect there probably because of some incompatibility between those modules, so I think we should skip that upgrade until we also do webpack.

@silverwind
Copy link
Member Author

It looks like there's actually no more blockers for webpack 5, monaco plugin does not specify compatibilty but seems to work, so I'll try putting that update in here.

@silverwind silverwind marked this pull request as draft December 23, 2020 10:12
@silverwind silverwind marked this pull request as ready for review December 23, 2020 10:52
@silverwind
Copy link
Member Author

Upgrade to webpack 5 is now included as well as some changes regarding nicer monaco language chunk filenames. The stats output has changed considerably and I've tried to replicate what we had before (e.g. just a list of output assets).

@silverwind
Copy link
Member Author

Prod build is failing, it looks like we're blocked by xz64/license-webpack-plugin#67 with webpack 5.

@silverwind
Copy link
Member Author

Switched to a different license generation plugin that is compatible with webpack 5. Output should be nearly identical but that plugin does not offer a way to manually add modules so octicons is currently not featured in the output.

@silverwind
Copy link
Member Author

Still some webpack issues.

@silverwind silverwind marked this pull request as draft December 23, 2020 12:10
@silverwind silverwind marked this pull request as ready for review December 23, 2020 13:16
@silverwind
Copy link
Member Author

Ok it's ready now. I had to overwrite one module for license generation because it had a non-compliant license field. Maybe we can later switch back to license-webpack-plugin once it's compatible with webpack 5.

@silverwind silverwind changed the title Update JS dependencies Update JS dependencies and webpack Dec 23, 2020
@CirnoT
Copy link
Contributor

CirnoT commented Dec 24, 2020

license-webpack-plugin should be compatible now https://github.com/xz64/license-webpack-plugin/releases

@silverwind
Copy link
Member Author

Found one more issue with it: xz64/license-webpack-plugin#89

@silverwind
Copy link
Member Author

silverwind commented Dec 24, 2020

I think we shouldn't be holding off this update until license-webpack-plugin is fixed as there may be yet another issue with that plugin where it misses some licenses. The current plugin works and the only issue I have with it is that it offers no way to manually add modules or disable the license checking, but those are minor.

Also, I'm not totally happy with the new webpack output as it's less readable than before, but I guess we can not really change that.

@silverwind
Copy link
Member Author

Switched back to license-webpack-plugin as the latest version is now working, at least to some extend. It is missing detection of a few modules as compared to previously, see xz64/license-webpack-plugin#91 for it. Not a blocker thought.

monaco-editor-webpack-plugin was updated as well which now officially states webpack 5 compat, even thought it was already working previously.

@silverwind
Copy link
Member Author

Everything working now, squashed and ready.

- Update all JS dependencies
- Adapt webpack config for version 5
- Update to Less 4.0, adapting usage of removed mixin syntax
- Enable new ESLint rules and fix discovered issues
@silverwind
Copy link
Member Author

Gone back to the other license plugin because it performs much faster.

license-webpack-plugin09:20: https://drone.gitea.io/go-gitea/gitea/34349/1/8
license-checker-webpack-plugin 04:01 - https://drone.gitea.io/go-gitea/gitea/34351/1/8

Overall, webpack 5 seems about 20% slower than webpack 4 but I guess it's something we have to accept for now.

@lunny
Copy link
Member

lunny commented Dec 27, 2020

@silverwind If it's slower, but an upgrade is really necessary?

@silverwind
Copy link
Member Author

Right now we could live with webpack 4 but it will eventually go out of support and if I recall correctly, webpack 5 will be required for certain things like #12459 because if offers additional options.

I plan to look into pre-building monaco which is the biggest chunk of webpack build time, which should then get us down to probably half of what it is now.

@silverwind
Copy link
Member Author

Actually the perf regression isn't so bad, maybe 10% and I expect it to become faster in future updates.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 27, 2020
@6543 6543 merged commit 50a2dd5 into go-gitea:master Dec 27, 2020
@silverwind silverwind deleted the deps-21 branch January 3, 2021 15:09
@azhai
Copy link

azhai commented Jan 7, 2021

Remove the dir named node_modules, and rebuild
删除 node_modules 目录,然后重新编译

rm -rf node_modules/
TAGS="bindata sqlite sqlite_unlock_notify" make build

@go-gitea go-gitea locked and limited conversation to collaborators Feb 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants