-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Conversation
Β Open in CodeSandbox Web Editor | VS Code | VS Code Insiders |
β Deploy Preview for nuxt3-docs canceled.
|
Nice find! Do you mind splitting this into a new fix/perf PR? I have some concerns about how we do bundle/size testing. It made honestly lots of trouble over time in Nuxt 2 and made the CI of Nuxt 2 fragile as each package can actually affect bundle size easily and hardcoded size was not a good idea. BTW I'm fine with making test limited to testing used node_modules in snapshot as the list changes much less often. |
Done. But for what it's worth, I think it is worth keeping a bundle size test. It takes 10s to update the snapshots for the bundle size, and at least that means that we are deliberately choosing to increase server or client bundle size rather than doing it accidentally. |
True but it quite often happens because an (expected as we test) external library simply adds new functionality and it adds a few bytes to the server or app bundle. Over time all libs and our internal codebase increase and it is by itself a deliberate choice of us or 3rd party library authors that make this choice. Bein required to manually update the test, is a duplicate effort unless it often would cause us to revise even if using/upgrading a library or not. I think the entire logic for size testing, could be an informational CI action similar to code coverage integration signaling us about certain percentage increase. There might be even some ready to use integrations... In the meantime i appreciate if we remove size cap logic from PR changes completely to merge and add it back as CI integration later. |
The logic is already removed unless running manually locally for ease of testing. π |
Size caps are still hardcoded though... Can we write sizes to (a gitginored file) for local testing? Later CI can pick same file as well and show in gh PRs. |
I agree, it would be nice to print results in CI. |
When originally pushed it was not. |
I've enabled back CI checks with hardcoded sizes. Let's hope with Nuxt 3 we require to updated the caps less often. Also let's track windows issue later. |
Or it was but didn't reach the cap size. something else could added few bytes and reveal it. Also note that I've changed the measurement to use actual byte size. It is different from how much filesystem of OS takes to store files to the disk. |
I saw that change. But the variability was not so high. The numbers were:
Whereas the difference in your last example is more like 250kB, which is a huge difference! |
Yeah that's strange! Specially that only windows is bigger. We could make different values between windows and linux for server bundle size.... |
Is it caused by reading byte size? Maybe this is the difference? Why should the size on disk have been the same, but byte size differ so much? |
I'm not suggesting it necessary was the reason. I think since then we also migrated to pnpm and upgraded couple of dependencies. Anyway since byte size difference now, i think something is really wrong probably with nitro externals in windows (not tests) π¬ |
π Linked issue
β Type of change
π Description
This adds a test to constraint bundle size growth for an out-of-the-box install. This will prompt us to update snapshots when/if new packages are added to build.
Any ideas for improving assertion or test more than welcome.
I've also fixed an issue where
vue-router
and@vue/devtools-api
were traced as dependencies even without router integration.π Checklist