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

Tree shaking issue in 2.17 #2207

Closed
tkat opened this issue Feb 18, 2021 · 33 comments · Fixed by #2339
Closed

Tree shaking issue in 2.17 #2207

tkat opened this issue Feb 18, 2021 · 33 comments · Fixed by #2339

Comments

@tkat
Copy link

tkat commented Feb 18, 2021

I'm using date-fns in two projects (without additional locales) and when I upgraded to 2.17 the bundle size went to ~200Kb.

Checked with the webpack bundle analyzer, and all the functions seem to be there. I'm actually using just 7 functions with import syntax (ESM), so nothing changed from my side. If I rollback to 2.16.1, everything is OK again.

Does it have to do with the Typescript addition? My projects are plain js.

@phil-lgr
Copy link

image

Same here, on 2.17

Here is my @babel/preset-env

image

I don't see why it wouldn't work. The module field is set correctly to date-fns/esm

@phil-lgr
Copy link

Ok, so im my case, I had another package that was using date-fns, but that package was wrongly set to commonjs output

image

@tkat
Copy link
Author

tkat commented Feb 19, 2021

Ok, so im my case, I had another package that was using date-fns, but that package was wrongly set to commonjs output

image

So, you must have had the issue with 2.16.1 too then, correct?

@phil-lgr
Copy link

I'm on 2.17, and with the set up above (@babel/preset-env) + tsconfig with "target": "es6",

and imports that looks like:

import { endOfDay, endOfMonth, isDate, parse, sub } from 'date-fns';

Treeshaking works correctly

@tkat
Copy link
Author

tkat commented Feb 19, 2021

It's weird that both my projects have the same behavior on 2.17, but I'll try to figure out if there's something similar to your problem.

Thanks for the reply and the pointers.

@tkat
Copy link
Author

tkat commented Feb 20, 2021

I think I am now closer to the issue. Full disclosure: I'm using the functional (fp) flavor of date-fns (which I failed to mention earlier).

So, long story short I created a vanilla project, and by importing like this, tree-shaking works correctly.
import { parse, format } from "date-fns"

But if I try to import the functional flavor of the same functions tree shaking never happens
import { parse, format } from "date-fns/fp"

image

So, importing them one by one solves the problem and tree shaking is working again

import parse from "date-fns/fp/parse";
import format from "date-fns/fp/format";

I still think there's an issue here though.

@jpduckwo
Copy link

jpduckwo commented Feb 22, 2021

Something has changed with the format of package in 2.17.0. I've had issues with my Angular 11 project and warnings being thrown that it's using a commonjs version of the date-fns package. Sticking with 2.16.1 for now which worked fine.

EDIT: my angular project is target: es5

@FallDownTheSystem
Copy link

Not sure if this is related, but for me import { format } from 'date-fns' only adds 20kb, but importing a locale
import { fi } from 'date-fns/locale' adds about 550kb.

@Louvki
Copy link

Louvki commented Feb 25, 2021

Same issue, it seems like it's importing every locale despite me requesting only 3

locales
Screenshot_2
locales3

@inker
Copy link

inker commented Mar 1, 2021

2.18.0: still the same issue

@martinhiller
Copy link

It looks like the package.json files of the subpackages do not contain references to the ESM variant anymore.
For instance, in node_modules/date-fns/format/package.json:

2.16.1

{
  "sideEffects": false,
  "module": "../esm/format/index.js",
  "typings": "../typings.d.ts"
}

2.17.0

{
  "sideEffects": false,
  "typings": "../typings.d.ts"
}

indeyets added a commit to FreeFeed/freefeed-react-client that referenced this issue Mar 5, 2021
Newer versions have a bug which leads to much larger js-chunks
date-fns/date-fns#2207
@MickL
Copy link

MickL commented Mar 9, 2021

I can confirm tree shaking works with 2.16.1 but in 2.19.0 full date-fns is imported

@bondarenko09
Copy link

Any updates for this? Have the same issue. I have created custom locales file for Ukrainian, English and Russian, with some custom translations. I want lazy load these files via webpack import(), each of these files contains import { [localeName] } from 'date-fns/locale', but when I build it, I get a separate chunk with all locales which size is about 3Mb

@fturmel
Copy link
Member

fturmel commented Mar 18, 2021

Specifically for locale tree-shaking, I think the culprit is this file that ends up exporting every locale explicitly: https://github.com/date-fns/date-fns/blob/master/src/locale/index.js

In my app (CRA, Typescript, date-fns 2.19), I managed to work around the issue by importing my locales slightly differently and bypassing that index:
import { frCA } from 'date-fns/locale'; // date-fns ~760 KB
import frCA from 'date-fns/locale/fr-CA'; // date-fns ~35 KB

I'm not completely sure if this is a webpack bug / new behavior? Maybe the official date-fns docs should encourage this alternative import method, or at least mention it in the ESM/Tree-shaking section?

Hope this is helpful!

EDIT:
I also noticed that importing from date-fns/esm instead of date-fns does work for me. ex:
import { differenceInYears, format, formatDistanceToNow, parseISO } from 'date-fns/esm';
import { frCA } from 'date-fns/esm/locale';

@fturmel
Copy link
Member

fturmel commented Mar 18, 2021

@martinhiller thanks to you, I think I figured out the root cause and just opened a pull request (#2339)

@antch
Copy link

antch commented Apr 9, 2021

Any update on this? The PR from @fturmel has been open and awaiting review almost as long as it took for them to open it in the first place.

@mlandalv
Copy link

Does the pull request really address the locales issue (I haven't tested)?
Isn't the problem that import { fi } from 'date-fns/locale' always imports from the commonjs export and not esm (esm/locale), and hence webpack can't tree shake.

So lets say you're using babel + webpack and are running in an esm environment.

import { format } from 'date-fns'; // <-- esm import because of package.json#module
import { fi } from 'date-fns/locale'; // <-- always cjs import

Then you have a mix of imports from both cjs and esm. And since tree shaking only works in esm the locales bundle is big.

I think the fix is the package.json "exports" field. That way the date-fns/locale imports can be context aware. Webpack 5 understands that field, not sure about v4.

This seems to work if added to package.json.

  "exports": {
    ".": {
      "import": "./esm/index.js",
      "require": "./index.js"
    },
    "./locale": {
      "import": "./esm/locale/index.js",
      "require": "./locale/index.js"
    },
    "./package.json": "./package.json"
  },

But since some people use subpath imports, those also needs to be added, e.g. "./parseISO": { ... } etc.

@fturmel
Copy link
Member

fturmel commented Apr 12, 2021

@mlandalv Yeah, I just verified again and can confirm that #2339 fixes this issue.

There is a build step that generates over a thousand nested package.json files and they were all missing their module property because of an async bug. You can't see these files on Github because they are created after the babel transpiling phase in the tmp directory.

See the bundle analysis in my project which only imports one locale:

With fix from pull request (date-fns 32KB)

with-fix

Version 2.20.2 (date-fns 786KB)

v2 20 2

@fturmel
Copy link
Member

fturmel commented Apr 12, 2021

@antch It's strange that this issue has been ignored considering how it must be significantly impacting the bundle size of tons of projects out there without people noticing. The modular structure / tree-shaking ability of the project is literally the first thing mentioned in their promotional material everywhere and currently that statement is not true anymore if you import from locale and fp.

Screen Shot 2021-04-12 at 1 11 12 PM

@kossnocorp
Copy link
Member

@antch, I would like to ask you to stop letting down contributors, especially when you didn't contribute anything to the project or issue.

@kossnocorp
Copy link
Member

@fturmel thank you for your fix. We have to process many incoming issues and pull requests, and unfortunately, this critical issue went unnoticed.

@antch
Copy link

antch commented Apr 13, 2021

I will admit this was not the correct forum to vent about that, especially since I even mentioned I am not sure if it even applies this project team. Apologies.

@kossnocorp
Copy link
Member

@antch, no worries, thank you for your reply!

@kossnocorp
Copy link
Member

I've shipped the fix with v2.20.3. Please upgrade and see if it helped.

@Freddixx
Copy link

Freddixx commented Apr 22, 2021

@kossnocorp I am still quite sure that something isn't working as it was intended to.

I am working on a project where I upgraded code from date-fns 1 to version 2 this week and the bundle sizes have really taken a turn for the worst.

The only migration change I made in that bundle is from:

import format from 'date-fns/format';
import parse from 'date-fns/parse';

to

import format from 'date-fns/format';
import parseISO from 'date-fns/parseISO';
// Also tried import { format, parseISO } from 'date-fns'

and it went from this:

to this:

I don't think that is intended behaviour. Not sure how many people here are thoroughly watching their bundle sizes but I'd be interested to know if somebody else is experiencing this too.

If you would like me to I can spawn a separate issue since the downgrade hasn't helped in my case either. But I felt this belonged here.

@mattleonowicz
Copy link

mattleonowicz commented Apr 22, 2021

@Freddixx check that 2.16.1 version. It was working for everyone.
If there is no difference, then I'd say it's safe to say that your issue is unrelated.

@Freddixx
Copy link

Freddixx commented Apr 22, 2021

As said above: Downgrading unfortunately didn't solve the issue for me.

@fturmel
Copy link
Member

fturmel commented Apr 22, 2021

@Freddixx I'm not super experienced with webpack-bundle-analyzer but I quickly set it up in my project to compare. Everything is looking good on my end with date-fns v2.21.1

Perhaps it's a project-specific issue (Webpack/Babel config), or simply a misreading of the analyzer output (the paths are different in your images)? I find it interesting that you don't have parsed and gzipped values in your screenshots like mine does, maybe that does hint to something missing in your build step. Are you seeing the same problem for other libraries with esm modules?

These might be worth reading:
https://github.com/webpack-contrib/webpack-bundle-analyzer#size-definitions
https://webpack.js.org/guides/tree-shaking/

Screen Shot 2021-04-22 at 12 52 04 PM

@Freddixx
Copy link

Yes, the paths are supposed to be different. Because the esm support is a date-fns v2 feature.

The missing gzip and parsed values are due our system architecture. It's a micro-frontend architecture where each component gets packed into its own bundle. For these "virtual" files the bundle analyzer can only output stat values.

But even if - that doesn't explain the difference between v1 and v2 - after all both scenarios operate under the same conditions 🤔

But really, any input is very much appreciated.

@Freddixx
Copy link

Freddixx commented Apr 26, 2021

I really hope I am wrong, but something is strange here @kossnocorp

What I did: I spawned a blank CRA (create-react-app) and loaded parse and format:

import format from 'date-fns/format';
import parse from 'date-fns/parse';

date-fns v1:

image

date-fns v2:

image

And since it was suggested that this might be a false reading by webpack-bundle-analyzer, I also went on and used source-map-explorer:

date-fns v1:

image

date-fns v2:

image

P.S.: Yes, I tested this on both the latest version and 2.16.1

@fturmel
Copy link
Member

fturmel commented Apr 26, 2021

@Freddixx I get what you’re saying, there certainly is a significant file size difference.

I’m not a date-fns team member and I might certainly be wrong here. I’m just trying to help and make sure my PR didn’t mess anything up. If anyone else wants to jump in feel free.

From what I can see, these two methods have changed quite a bit under the hood in v2. The file size difference might simply come from a larger source code implementation and depending on more internal methods. I’m sure there are differences in features and robustness that may not be apparent from the outside.

https://github.com/date-fns/date-fns/blob/v1.30.1/src/format/index.js
https://github.com/date-fns/date-fns/blob/master/src/format/index.js

https://github.com/date-fns/date-fns/blob/v1.30.1/src/parse/index.js
https://github.com/date-fns/date-fns/blob/master/src/parse/index.js

In v2, you have a few parse and format methods to choose from. If you can for example use formatISO and parseISO instead in your project, they bring much less complexity and I think the total size would be closer to what you observed with v1.

I also stumbled on this comment in the source code: The previous parse implementation was renamed to parseISO.
https://github.com/date-fns/date-fns/blob/master/src/parseISO/index.js#L36-L43

So, in my opinion this is unrelated to #2207 and tree-shaking works as intended now.

I do however think that you bring an interesting point. Maybe you could propose an effort to reduce the file size of the parse and format methods in a new issue? See if it gains any traction and if it’s even feasible? Or perhaps some additional documentation on v2 migration could’ve helped? If some methods can add 20KB to your bundle size while others only a few, maybe this information could be documented to manage expectation?

@danielo515
Copy link

I think I am now closer to the issue. Full disclosure: I'm using the functional (fp) flavor of date-fns (which I failed to mention earlier).

So, long story short I created a vanilla project, and by importing like this, tree-shaking works correctly.
import { parse, format } from "date-fns"

But if I try to import the functional flavor of the same functions tree shaking never happens
import { parse, format } from "date-fns/fp"

So, importing them one by one solves the problem and tree shaking is working again

import parse from "date-fns/fp/parse";
import format from "date-fns/fp/format";

I still think there's an issue here though.

This fixed the problem for me on 2.19.0. Thank you very much!!

@liby
Copy link

liby commented Feb 27, 2023

I also noticed that importing from date-fns/esm instead of date-fns does work for me. ex:
import { differenceInYears, format, formatDistanceToNow, parseISO } from 'date-fns/esm';
import { frCA } from 'date-fns/esm/locale';

Test results:

// 124.6 KB
import { enUS } from "date-fns/esm/locale";

// 26.1KB
import enUS from "date-fns/esm/locale/en-US";

// 26.1KB
import enUS from "date-fns/locale/en-US";

image

image

image

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.