-
Notifications
You must be signed in to change notification settings - Fork 91
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
feat: Move to vite for bundling #5367
Conversation
3b4b9fe
to
93fc058
Compare
93fc058
to
24903e7
Compare
9081a0f
to
0ed9c1c
Compare
Build is also failing for me locally with
|
|
in public read only shares prosemirror.scss is not getting applied (did not look at editable shares yet) |
44def8c
to
9da2f3f
Compare
Signed-off-by: Julius Härtl <[email protected]> Revert "fixup! fixup! fixup! feat: Move to vite for bundling" This reverts commit d3c2ac5.
Signed-off-by: Julius Härtl <[email protected]>
Signed-off-by: Julius Härtl <[email protected]>
Signed-off-by: Max <[email protected]>
to fix node builds for now. Rollup is known to require quite a bit of memory when bundling with source maps enabled. https://rollupjs.org/troubleshooting/#error-javascript-heap-out-of-memory Signed-off-by: Max <[email protected]>
Signed-off-by: Max <[email protected]>
took the `configFile: false` from nextcloud/forms. Signed-off-by: Max <[email protected]>
Signed-off-by: Max <[email protected]>
Looks like we were just being lucky and rules were in the right order before. With vite including the css in tags we cannot rely on this.--signoff Signed-off-by: Max <[email protected]>
Preview generation locks the file leading to occasional 423 responses on the delete request. Signed-off-by: Max <[email protected]>
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.
I think this is ready to go now. Would love to get another pair of eyes on the changes I made myself though.
@juliushaertl I cannot ask you for review because it's your PR. Would you take a look and merge if you think it's ready? |
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.
Awesome, thanks again for taking over and pushing this forward. 👍
Improves js assets quite a lot, while more individual files are loaded the overall size by ~80%
Initial files app load
Before 17.43MB
After 3.94MB