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

"Cookies" library causes a significant increase of vendors bundle size #330

Closed
remychvn opened this issue Jun 21, 2019 · 4 comments
Closed

Comments

@remychvn
Copy link

remychvn commented Jun 21, 2019

What problem does this feature solve?

Reducing client bundle size to improve website load time

Hi,

I noticed that the addition of nuxt-i18n increased a lot my vendor.app.js bundle size
I looked for the source of this significant increase.
It seems that it's caused by the import of https://github.com/pillarjs/cookies that calls nodejs functions. (http and crypto)
Due to this, webpack adds many libraries to vendor bundle. Is this really necessary on the client side, just for a cookie library? I didn't dig deeper into the codebase to affirm that.
Screenshot from 2019-06-22 01-43-38.png

I dont know if you can do something to prevent this or if we must extend the nuxtjs webpack configuration ? How ?

This feature request is available on Nuxt community (#c251)
@ghost ghost added the cmty:feature-request label Jun 21, 2019
@paulgv
Copy link
Collaborator

paulgv commented Jun 22, 2019

@rclement WDYT? I knew that new library was be a bit bigger than the previous one but I wouldn't expect a big difference in bundle size. Would it be worth it keeping the old library and refactoring the code to still address the issue while keeping the bundle size as light as possible?

@rclement
Copy link

@paulgv Indeed that’s a bit crazy just for handling cookies properly, I admit I was in a hurry and only checked my project bundle where Cookies may have already been included by another dependency...

Of course, I’m all for a rewrite, as long as the previous bug is not reintroduced. Maybe reusing the ˋcookieˋ package and manually handling the ˋSet-Cookieˋ header properly is the way to go? We can be inspired by the startup of the ˋcookies` package which does just that (handling whethet this is undefined, a string or already a list).

I won’t have time for it this week-end but I can do it next week if needed.

@rchl
Copy link
Collaborator

rchl commented Jun 22, 2019

The answer to question in #329 also applies here. cookies library is not needed on the client so it could be required conditionally and then it wouldn't even be included on the client.

@rclement
Copy link

@paulgv @remychvn I under-estimated my available time for this week-end, so I manage to send a new PR (#332) hopefully making the best of both worlds:

  • Revert to using the cookie package to avoid increase of vendor bundle size
  • Still fixing the original bug by manually handling the Set-Cookie headers, in the same fashion as the cookies package does

Let me know if missed something, again.

@paulgv paulgv closed this as completed in 9cd034d Jun 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants