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

size is TEN TIMES BIGGER since including turf #228

Closed
codeofsumit opened this issue Sep 22, 2017 · 12 comments
Closed

size is TEN TIMES BIGGER since including turf #228

codeofsumit opened this issue Sep 22, 2017 · 12 comments

Comments

@codeofsumit
Copy link
Contributor

codeofsumit commented Sep 22, 2017

turf is the dependency to calculate the necessary things to support the Cutting feature.
Turf thinks about replacing it's big dependency that's causing this size: Turfjs/turf#88

image

Another option would be to replace turf with martinez. Martinez still has some problems though and I wasn't able to make holes work with it due to this: w8r/martinez#44

Afaik, turf thinks about replacing their big dependency jsts with martinez too which would be my favorite option.

Let's see 🤞

@DenisCarriere
Copy link
Contributor

👍 💯 support this, feel free to send PR's to both libraries, TurfJS is also looking forward to implement martinez.

Also we're in the process of converting to ES6 modules to enable bundlers like Rollup to be more efficient (🤞 coming soon... Turfjs/turf#960)

@codeofsumit codeofsumit mentioned this issue Oct 9, 2017
@codeofsumit
Copy link
Contributor Author

@ivanjaros asked if we could exclude turf if the cutting feature is not used.

Here is what I'm currently thinking about:

  1. Split leaflet.pm into modules to allow separate import of features to reduce file size
  2. wait for turf to replace jsts
  3. replace turf (I did not find a suitable replacement yet)

I'm in favor of 2. or 3. and I'll start searching for a replacement again this week as turf moved the remove jsts milestone behind the next major version release (5.0).

In apps where I use leaflet.pm myself I started lazy loading it. I mostly use Webpack and React / Vue. If there is a Map component of some sort, it's VERY easy to lazy load it so leaflet and leaflet.pm are not in the initial bundle to be downloaded.

@codeofsumit
Copy link
Contributor Author

Maybe the recent merge of Turfjs/turf#960 helps here with it's released in Turf 5.0?
Turf 5.x seems to be released on Github
https://github.com/Turfjs/turf/releases

But not on npm?
Will investigate and if the turf update brings an improvement to our filesize I'll release asap!

@DenisCarriere
Copy link
Contributor

@codeofsumit Still a few more things that need to be done before we release Turf 5.0 on npm. At the moment there's a few modules that are published for testing purposes.

@themre
Copy link
Contributor

themre commented Nov 23, 2017

seems it's now nice to move to new version. here I see only 2.61 for intersect: https://yarnpkg.com/en/package/@turf/intersect

@DenisCarriere
Copy link
Contributor

DenisCarriere commented Nov 25, 2017

Almost there... posted a #255 (comment) which might solve the issue very soon.

🤞

@smeijer
Copy link

smeijer commented May 1, 2018

This is the only thing I don't like about migrating from leaflet-draw to leaflet.pm.

leaflet-draw:   67.56 kB      https://unpkg.com/leaflet-draw@1.0.2/dist/
leaflet.pm:    421.64 kB      https://unpkg.com/leaflet.pm@0.24.0/dist/

@codeofsumit
Copy link
Contributor Author

codeofsumit commented May 1, 2018

@smeijer Turf already brought down intersect.
Now it's only @turf/difference that has no 6.x version on npm that is still big. If we can replace or update that, this library will go down to 80kb.

image

@DenisCarriere is there a specific reason that difference has no 6.x version on npm? Github also has no 6.x releases. I'm a bit confused by turfs releases/versioning 🤔

@smeijer
Copy link

smeijer commented Jun 13, 2018

According to NPM, @turf/difference received an update to 6.x.

https://www.npmjs.com/package/@turf/difference/v/6.0.1

I hope this also triggers the review of some PR's, and continued development of this awesome leaflet.draw alternative.

@DenisCarriere
Copy link
Contributor

DenisCarriere commented Jun 13, 2018

@smeijer @codeofsumit @turf/difference is updated to 6.x and doesn't contain any large dependencies.

Maybe make sure you refresh your yarn.lock or any dependencies that might still have an older version of it.

https://unpkg.com/@turf/difference@6.0.1/

You need to UPDATE =>

yarn.lock
https://github.com/codeofsumit/leaflet.pm/blob/develop/yarn.lock#L27-L35

package.json
https://github.com/codeofsumit/leaflet.pm/blob/develop/package.json#L22

@DenisCarriere
Copy link
Contributor

Fixed via PR #300

@codeofsumit
Copy link
Contributor Author

The turf team did it 🎉

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

No branches or pull requests

4 participants