Skip to content
This repository has been archived by the owner on Apr 1, 2020. It is now read-only.

[DNM] replace moment w/ date-fns #7

Closed
wants to merge 1 commit into from
Closed

[DNM] replace moment w/ date-fns #7

wants to merge 1 commit into from

Conversation

tomwayson
Copy link
Owner

Currently inlining date-fns using rollup plugins.

Prob is that the source for the 3 functions we use from data-fns is 3x the size of the source of this repo:

> rollup -c profiles/dev.js

src/index.js:
date-fns - 15.48 KB (73.17%)
app - 4.97 KB (23.49%)
rollup helpers - 723 B (3.34%)

I'm sure a lot of that is comments, etc, b/c the final output is reasonable:

Destination: dist/opendata-chart-utils.min.js 
Bundle size: 4.41 KB, Gzipped size: 1.71 KB   

Not totally convinced this is a better approach than treating moment as an external - only makes sense if we can get rid of moment in OD, and we don't end up using a whole bunch of date-fns in OD. Alternatively we could try to figure out how to use date-fns as an external.

@tomwayson tomwayson requested a review from benstoltz March 18, 2017 15:16
@tomwayson
Copy link
Owner Author

tomwayson commented Mar 19, 2017

For reference, that's about 2x the size of an uglified build of master:

Destination: dist/opendata-chart-utils.min.js 
Bundle size: 1.71 KB, Gzipped size: 755 B

@tomwayson
Copy link
Owner Author

SIgh. Looks like, should we choose to merge this, we'll need this yarnpkg/yarn#1776 (comment)

@tomwayson
Copy link
Owner Author

Closing this, as Open Data is hopelessly tied to moment

@tomwayson tomwayson closed this Apr 28, 2017
@tomwayson tomwayson mentioned this pull request Apr 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant