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

Change documentation to avoid loading *all* locales in generated bundles #2188

Closed
pounard opened this issue Feb 5, 2021 · 10 comments · Fixed by #2339
Closed

Change documentation to avoid loading *all* locales in generated bundles #2188

pounard opened this issue Feb 5, 2021 · 10 comments · Fixed by #2339

Comments

@pounard
Copy link

pounard commented Feb 5, 2021

Hello !

First of all, thank you for all your work. I'm not a direct user of this library, but a dependency I use uses it, and it works fine.

When I compiled (using Webpack via symfony/webpack-encore, typescript and vue-loader, which is a rather common setup) I had the very bad surprise, after doing some analysis, to find out that date-fns was responsible of 1.7M in the whole 3M full bundle. 900k were because of the locales.

I was using this code to import my locale:

import { fr } from 'date-fns/locale';

And I converted it to:

import fr from 'date-fns/locale/fr';

And now it's not loading all locales anymore, only the one I need, and my bundle approximately halved in size.

Problem is, I read your documentation on the front page, which gives this code sample:

import { formatRelative, subDays } from 'date-fns'
import { es, ru } from 'date-fns/locale'

formatRelative(subDays(new Date(), 3), new Date())
/// ...
import { fr } from 'date-fns/locale';

You may consider change it to this variation:

import { formatRelative, subDays } from 'date-fns'
import es from 'date-fns/locale/es'
import ru from 'date-fns/locale/ru'

formatRelative(subDays(new Date(), 3), new Date())
// ...

It's a bit more verbose, but it will avoid bundlers to bundle all locales, and only embed the requested ones instead, and save previous internet bandwidth :) The main problem with your code sample on the front page is that it will be the very first that new users will see, and they will, like I did, copy/paste it naively in most case.

Thanks again for all your work.

@chriscarpenter12
Copy link

I can confirm the same findings within an Angular library/app. We have an Angular library we utilize date-fns/locale imports from. Previously we used the import method described in the documentation and upon inspecting webpack bundles of our app builds indeed all locales are included in the app. Not just the ones we use within the library. Modifying the imports to be like import es from 'date-fns/locale/es' fixes the unused import issue.

Secondly if anyone else finds this. Changing this also fixes another issue during a production Angular build with aot and buildOptimizer both enabled. Previously using the recommended import path the barrel index file at date-fns/locale/index.js is shaken out of a build, and all locale reference were undefined. Even though the locales were indeed present in the final bundle. Just no barrel index file to import from.

@pounard Thanks for point me toward a fix!

@psoares-resilient
Copy link

Our built app also started failing on the 2.17.0. A single place used:

import { enUs } from 'date-fns/locale

causing the runtime error:
ERROR TypeError: Cannot read property 'localize' of undefined.

Reverting to 2.16.1 "fixed it". Was about to report it when I found this thread and tried out the fix by @pounard switching to:

import enUs from 'date-fns/locale/en-US

It all started working again.

I don't really understand why this works, but it does. If anyone has clarity on why 2.16.1 is fine (even though it might bundle everything) and 2.17.0 just plainly causes the error above I'm interested in understanding why.

If it helps, we are using "@angular/core": "^10.2.4", and this is part of a custom dateTime adapter for the OwlDateTimeModule from "ng-pick-datetime": "^7.0.0",

@pounard
Copy link
Author

pounard commented Feb 26, 2021

Ok, so I may have mislead everyone, I'm sorry, but when importing the locale as such:

import fr from 'date-fns/locale/fr';

It doesn't work, it doesn't yield any errors thought (and I have no idea why).

EDIT: to be more clear, when importing the fr object directly from the fr module, text remains in english instead of being translated.

Help would be appreciated from library maintainers here.

@pounard
Copy link
Author

pounard commented Mar 18, 2021

Still no date-fns contributor has laid eyes on this issue ?

@Haraldson
Copy link

@pounard This comment might be relevant for you: #1697 (comment)

@pounard
Copy link
Author

pounard commented Mar 24, 2021

@pounard This comment might be relevant for you: #1697 (comment)

Hum I think not, problem doesn't seem to be the same, as I don't get any webpack errors.

@Haraldson
Copy link

Well, this is about not bundling any other locales than the ones you support, right? That's what that snippet does. It doesn't solve it in the particular way you're suggesting, but it is one way of achieving the same result.

@pounard
Copy link
Author

pounard commented Mar 24, 2021

It doesn't solve it in the particular way you're suggesting, but it is one way of achieving the same result.

Maybe, I'll look at it once again.

One thing that bites me when reading this, is that it's non trivial webpack conf, and terribly unreadable (and hardly maintainable). I'm not comfortable with node module resolution JS is not my domain of expertise, yet I have the hunch that if:

import { fr } from 'date-fns/locale';

is working, then:

import fr from 'date-fns/locale/fr';

should be as well.

The fact that it's not is not trivial to me, my guess is that it in an ideal world this would work transparently, and the fact it doesn't might be a bug in the way you export those locales. I would be happy to help you fix it, but I'm no JS expert, I'd very much like help from this library maintainers so that it would be fixed in a way we wouldn't have to mess up badly with webpack conf.

Just for your information, I'm not using webpack directly, but Symfony's webpack-encore webpack auto-configurator. I already had to customize generated webpack configuration in the past, it's a mess, not because of encore, but mainly because of webpack itself. It would be much nicer to all people if date-fns was behaving in a way that we wouldn't have to customize any bundler configuration.

Thanks for your time, I hope you will investigate this.

@fturmel
Copy link
Member

fturmel commented Apr 12, 2021

For everyone in this thread, #2207 overlaps with this issue. It's a real bug with a PR pending, so I don't believe there would be a need to update the documentation.

@tan75
Copy link
Contributor

tan75 commented Apr 13, 2021

hi all,
the fix is on the way - see PR2339

@tan75 tan75 closed this as completed Apr 13, 2021
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

Successfully merging a pull request may close this issue.

6 participants