luxon icon indicating copy to clipboard operation
luxon copied to clipboard

Better Documentation regarding Size and Tree-Shaking

Open noahgrant opened this issue 3 years ago • 22 comments

Hello!

This follows from from https://github.com/moment/luxon/issues/703 and https://github.com/moment/luxon/issues/94#issuecomment-346481639.

So, I have just moved from moment to luxon for the reasons listed on the moment homepage. When I read about using native APIs in more modern libraries or new versions of old libraries, I guess I assume that the new library will have a size savings, as well. Imagine my surprise, then, when I finally migrate everything over and find out that my vendor.js production-built file size has increased by ~~170kB~~ minified and ~~53kB~~ gzipped!! (Update: the comparison I did was not apples to apples; the more accurate increase is 18kB and 4kB, respectively.)

Had I known this ahead of time, I probably would've stuck with moment or moved to another library like Day.js (I still might). I'm currently not using moment's locales or its timezone addition, and at another company, I provided my own short-list of timezone data to keep moment's size down. In a similar manner, I'm not using the bulk of what Luxon exports (ie Interval and Zone), but since it's not modular I can't tell webpack, for example, to leave certain modules out.

I've read discussions on things people have tried in attempt to make their file sizes smaller, but what I'd really like is a consensus on what to do for each of the popular build tools/environments, and have that be listed in the Luxon documentation page. For example, how can I point my build to use the smaller global-es6/luxon.min.js instead of cjs-browser/luxon.min.js that it's currently using?

Additionally, I think it'd be really helpful to be upfront about the size costs of using Luxon (especially that you are comparing it with the fully-loaded moment library). One ideal place would be in the moment homepage under the Reasons to keep using Moment section that discusses Luxon—knowing that library size does not actually get smaller but rather increases would be really helpful to know when comparing alternatives.

Thank you for reading!!

noahgrant avatar Jan 12 '21 23:01 noahgrant

170kb is big in 2021? How about utilizing the browser cache with versioning? I guess it is negligible.

tcsaddul avatar Jan 13 '21 07:01 tcsaddul

170Kb would be a lot... but 24 Kb is ok.

@tcsaddul While 170Kb doesnt sound like a lot, it actually matter once things start to add up. If every libary u use would increase its size by 170Kb, you would increase the loadings times by a lot... even in 2021. In worst case, you can loose a lot of users, just because of increased loading times. It depends on the project ofc, but overall its not something that you shoulnd't care about, if you're creating a website on a professional level.

lephro avatar Jan 15 '21 21:01 lephro

If you are using import { ... } from 'luxon/src/luxon'; instead of import { ... } from 'luxon';, the size will be about 50Kb smaller. This also works with @types/luxon. - tested with Angular

MarcelCoding avatar May 08 '21 13:05 MarcelCoding

@MarcelCoding can you give an example? Nothing in the luxon repo seem to correspond with the function names I need to use (startOf, endOf, etc).

I'm encountering this same issue, I thought the entire point of redoing moment was to improve tree shaking, I just made a huge amount of changes and there isn't even tree shaking at all that I can find?

9mm avatar May 11 '21 01:05 9mm

@9mm What do you mean with an example? A real code example with size difference between the tow types of imports? Or what do you mean?

MarcelCoding avatar May 11 '21 08:05 MarcelCoding

No, I just would like to see an example of tree shaking, and how you would write the import statement.

Say for example I have this:

const date = DateTime.fromObject({zone: 'America/New_York'}).startOf('day');
date2 = date.plus({days: 1});

How would you write the imports for these functions so that tree shaking can occur? I've poked around the files and see nothing usable less it size than import { DateTime } from 'luxon';... but maybe I'm just not seeing it.

9mm avatar May 11 '21 15:05 9mm

export function test() {
  console.log('test');
}

export function hallo() {
  console.log('hallo');
}

Other file:

import { test } from '...';

test();

Here the function hallo should be removed. But in luxon this does not rely work. I think there are some dependencies between all methods/objects, that causes that that not many methods can be removed.

MarcelCoding avatar May 11 '21 15:05 MarcelCoding

@MarcelCoding I mean I understand how imports work, I'm saying specifically related to the luxon library. This entire ticket is about tree shaking being unclear in the documentation. You said it supports treeshaking is supported.

The entire reason I moved over to this library from date-fns was becuase their timezone support is truly awful, but low and behold after I convert everything there's no tree shaking to be found. You mentioned in this ticket how it's possible so I'm trying to figure out how exactly it is possible, ideally if you could show an example.

9mm avatar May 11 '21 15:05 9mm

Ah I see, you are saying to simply do this...

import { DateTime } from 'luxon/src/luxon';

9mm avatar May 11 '21 15:05 9mm

@9mm I discovered luxon only a short time ago. After working with luxon I noticed the problem with the bundle size. I found this issue on GitHub and just wanted to share my experience, as it might be helpful for other people. However, I can also say that something is going terribly wrong here, or perhaps almost the entire library is needed to perform simple operations.

MarcelCoding avatar May 11 '21 16:05 MarcelCoding

Maybe @icambron could help to clarify some things?

9mm avatar May 11 '21 16:05 9mm

One thing I should point out that may be helpful is that tree-shaking really isn't inherent to a single library. Yes, you need to do:

import {DateTime} from 'luxon';

instead of

import * as Luxon from 'luxon';

But that still won't help unless your build-tool supports tree-shaking, and to be honest, even though I use Webpack and it supposedly supports tree-shaking, the list of things required in order to get it to work are fuzzy at best and I have never actually gotten it to work. An alternative, which I mentioned in my original comment, is to use webpack's IgnorePlugin to leave out modules in the build. Because Luxon is not modular, I can't actually do this.

For me, what I wished I knew in advance with Luxon is that 'timezone support is built-in'—it's much, much smaller than using moment with timezone because it takes advantage of the native Intl API, but it's also bigger than using moment alone with no timezone at all (because for moment, timezone is opt-in). I'm not currently using timezone, so my size actually increased, which was unexpected, and I think Luxon could do a better job at explaining that upfront.

I may end up using timezones later, though, in which case I will be happy that I switched libraries.

noahgrant avatar May 11 '21 16:05 noahgrant

I will definitely say the way luxon handles timezones are A++. After using something like date-fns-tz which makes me die inside.

9mm avatar May 11 '21 16:05 9mm

I've long had on my todo list to simply experiment with this, and perhaps make changes that enhance the tree shakability of the lib. Unfortunately, I just haven't gotten to it

icambron avatar May 19 '21 16:05 icambron

I'm not sure about the reason, but it seems like luxon isn't tree-shakeable. It can be wrong build options or the way how library is written - in fluent-api way.

Tried with:

import { DateTime } from 'luxon' // the only import from luxon
import { merge } from 'remeda' // the only import from remeda

[...]
		this.expire = DateTime.now().plus({
			seconds: expires_in - 5 // slack
		})
[...]
		console.log(merge({}, {}))
		if (DateTime.now() > this.expire) {
			return true
		}

It seems like it works for merge:

function sr(t,e){return Object.assign({},t,e)}

[...]

function rr(t,e,n){var r=t.length-e.length,s=Array.from(e);if(0===r)return t.apply(void 0,s);if(1===r){var i=function(e){return t.apply(void 0,function(){for(var t=0,e=0,n=arguments.length;e<n;e++)t+=arguments[e].length;var r=Array(t),s=0;for(e=0;e<n;e++)for(var i=arguments[e],a=0,o=i.length;a<o;a++,s++)r[s]=i[a];return r}([e],s))};return(n||t.lazy)&&(i.lazy=n||t.lazy,i.lazyArgs=e),i}throw new Error("Wrong number of arguments")}

[...]

(console.log(function(){return rr(sr,arguments)}({},{})),er.now()>this.expire)

and it's not shown in the source-map-explorer. At least I can't find it without looking into the source map deeper. But at the core, there is no other code from remeda.

On the other side, luxon, give me this picture: image

If I use

import { Duration } from 'luxon' // the only import from luxon
[...]

		console.log(Duration.fromMillis(100000))

instead of above, it gives me a very similar picture: image

There is almost no difference.

If it's matter, I use webpack.

Bessonov avatar May 24 '21 12:05 Bessonov

I was thinking about moving from dayjs which is foresaken to Luxon but I believe this tree shaking thing is a serious issue. Can maintainers speak to state whether or not tree shaking is possible and how they plan to improve it in the future ? Are you working on this alone @icambron ?

JesusTheHun avatar Sep 17 '21 14:09 JesusTheHun

I was thinking about moving from dayjs which is foresaken to Luxon but I this tree shaking thing is a serious issue. Can maintainers speak to state whether or not tree shaking is possible and how they plan to improve it in the future ?

@JesusTheHun Can you explain what you mean by dayjs being forsaken?

ralfstory avatar Sep 17 '21 15:09 ralfstory

@JesusTheHun Can you explain what you mean by dayjs being forsaken?

Several core issues regarding Typescript support and ES6 are simply ignored for more than a year. PR for casual stuff are still merged but clearly the maintainer has other priorities - which is fine I mean it's FOSS but I'm looking for something robust for the years to come ; I would rather pay commercial support than having a lib already obsolete at the time of adoption.

Edit : it's even worse than I remembered, at the time of writing there are 41 PR open and 266 issues.

JesusTheHun avatar Sep 17 '21 16:09 JesusTheHun

In 2022, ESM imports were introduced. Would this allow tree shaking?

I find lots of things out there saying Luxon does not support tree shaking with webpack (and now rspack). But most is around 2021

tamusjroyce avatar Sep 21 '23 10:09 tamusjroyce

Looking at the published code, I believe tree shaking should work. However, due to many code pieces being used by other code pieces and the extensive use of classes (both are not inherently bad!), the benefits of tree shaking are very limited:

import { DateTime } from 'luxon'
console.log(DateTime.now())

image

Bessonov avatar Sep 21 '23 11:09 Bessonov

Any progress on better tree shaking for the near future?

hatsantos avatar Apr 15 '24 09:04 hatsantos