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

feat: Move to vite for bundling #5367

Merged
merged 10 commits into from
May 17, 2024
Merged

feat: Move to vite for bundling #5367

merged 10 commits into from
May 17, 2024

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Feb 6, 2024

  • Remove webpack config
  • Clean up dependencies

Improves js assets quite a lot, while more individual files are loaded the overall size by ~80%

Initial files app load

Before 17.43MB

Screenshot 2024-02-06 at 18 47 09

After 3.94MB

Screenshot 2024-02-06 at 18 45 45

@max-nextcloud
Copy link
Collaborator

Build is also failing for me locally with FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory when rendering chunks (466). Looks like one of the last chunks is too big or so.

--sourcemap false let's the build succeed.

@max-nextcloud
Copy link
Collaborator

NODE_OPTIONS: --max-old-space-size=4096 also works.

@max-nextcloud
Copy link
Collaborator

in public read only shares prosemirror.scss is not getting applied (did not look at editable shares yet)

@max-nextcloud max-nextcloud force-pushed the enh/vite branch 4 times, most recently from 44def8c to 9da2f3f Compare May 14, 2024 12:53
juliusknorr and others added 8 commits May 15, 2024 09:54
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]>
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]>
took the `configFile: false` from nextcloud/forms.

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]>
Copy link
Collaborator

@max-nextcloud max-nextcloud left a 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.

@max-nextcloud
Copy link
Collaborator

@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?

Copy link
Member Author

@juliusknorr juliusknorr left a 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. 👍

@juliusknorr juliusknorr merged commit e2d6ea7 into main May 17, 2024
61 of 63 checks passed
@juliusknorr juliusknorr deleted the enh/vite branch May 17, 2024 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants