Feature/bump deps

[themre]

I bumped dependencies and fixed webpack config so it works with latest stuff. also, probably due to new versoin of turf, we reduced size from 1.3 MB to 540 kB 🎉 tried a little bit and everything seems to work.

[codeofsumit]

ha! Awesome! I tried to do the same yesterday and failed at the webpack config
Still, the bundle size is not lower.
This is the latest leaflet.pm.min.js on the CDN:
https://unpkg.com/leaflet.pm@0.22.0/dist/leaflet.pm.min.js

It's 510KB.

Turf accounts for around 450KB in that . And afaik, turf did not replace the reason for that (JSTS) with a smaller library yet => sadly no bundle size improvement.

Will still merge this though thanks for this!!!

FYI: When turf finally moves from JSTS to martinez, leaflet.pm should be around 60 - 80KB.

[themre]

hm, strange because before bumping turf to 5.x I had 1.3 MB :D perhaps the problem is, that these modules are all connected and it adds that bundle size. I could try with different lib.

[codeofsumit]

The 1.3MB is unminified afaik.
If you're able to replace turf with other smaller libraries that do the two big functions that I use from turf (difference and intersect) it'd make a huge difference. Will be hard to find good ones though

[themre]

should we try rollup?

[codeofsumit]

We can but I don't think it makes a difference and here's why:

leaflet.pm is 500KB

• leaflet core: 50KB
• turf: 450KB

If Rollup also reduced the size of dependencies - we can try it.
But if turf is already bundled with Rollup, what difference does it make for us? We'd still get 450KB from turf that was already bundled by Rollup. If our Rollup implementation would make that smaller it would be very weird.

What do you think?

[codeofsumit]

@DenisCarriere can you chim in here? Can you give us more info if Rollup in our build would reduce the Turf build size? Or is Turf already bundled and optimized by Rollup?

[themre]

another thing I noticed is leaflet already has intersects method and they have ES modules.

[DenisCarriere]

At the moment the JSTS library isn't working perfectly to perform the Tree shaking properly. I'm currently working on refactoring JSTS for the sole purpose of adding it in TurfJS to minimize the code base and to allow the Tree Shaking to happen.

We should be able to push that release out at the next minor release.

@turf/intersect uses JSTS, so this will have the same Tree Shaking issue I mentioned.

Right now Rollup isn't doing a great job at handling JSTS because of extend.js & inherits.js which makes a copy of the Class thus making the repo even larger.

[DenisCarriere]

@codeofsumit How many TurfJS modules are you using in leaflet.pm? I can try to make sure those modules for the next minor release are refactored to properly support Rollup.

[themre]

"turf/difference": "^5.0.4",
"turf/intersect": "^5.0.4",
"turf/kinks": "^5.0.4"
thanks for help!

[DenisCarriere]

which ones?

Code block the JSON object

[themre]

i updated the comment, sorry. difference, intersect and kinks. I see there is also a bug with kinks: Turfjs/turf#1094

[DenisCarriere]

The 🐛 bug in kinks is only for 1 test case, it's still fine to use for most scenarios.

[themre]

@DenisCarriere btw, i saw that you mentioned somewhere that this is one way to import jsts features without turf, but need to use rollup: https://github.com/bjornharrtell/topolis/blob/master/src/utils.js. or am I mistaken?

[themre]

I started a PR with rollup (#256), some size descrease is noticed, however only with unminified version, because there is the same bug as here: Turfjs/turf#1086

[DenisCarriere]

@themre I've just published a new refactored version of JSTS which drops the extend.js (the cause of the error).

Try using this with Rollup and your configuration to see if you are still experiencing the same issues.

$ npm install turf-jsts

You can use @turf/intersect's source code as an example of how to use jsts, just need to replace jsts-es with turf-jsts (will most likely be renamed to jsts-es if it's stable)

import { GeoJSONReader, GeoJSONWriter, OverlayOp } from 'turf-jsts';

function intersect(poly1, poly2) {
    var geom1 = getGeom(poly1);
    var geom2 = getGeom(poly2);

    var reader = new GeoJSONReader();
    var a = reader.read(geom1);
    var b = reader.read(geom2);
    var intersection = OverlayOp.intersection(a, b);

    // https://github.com/Turfjs/turf/issues/951
    if (intersection.isEmpty()) return null;

    var writer = new GeoJSONWriter();
    var geom = writer.write(intersection);
    return feature(geom);
}

[themre]

thank you very much!

[themre]

I've tried with turf-jsts, but now I get this error:
[!] (uglify plugin) Error: Error transforming bundle with 'uglify' plugin: Unexpected token: name (NumberUtil)
I'll try to find some debug log to give some info.

[DenisCarriere]

Well turf-jsts is pure ES6, so you'll need to use Babel before you apply uglify, but I can look into it as well to see where it's coming from.

[themre]

I already have babel:

babel({
      presets: [['env', {
        modules: false,
        targets: {
          browsers: ['> 2%']
        }
      }]],
      exclude: [
        'node_modules/**',
        '*.json'
      ],
      babelrc: false
    })

[DenisCarriere]

Do you have a specific line of code that's causing it? can you expand the error a bit more?

I don't see much NumberUtil code that would cause an error:

https://github.com/DenisCarriere/turf-jsts/search?utf8=%E2%9C%93&q=NumberUtil&type=

[themre]

unminified version produces this:

class NumberUtil {
  interfaces_ () {
    return []
  }
  getClass () {
    return NumberUtil
  }
  equalsWithTolerance (x1, x2, tolerance) {
    return Math.abs(x1 - x2) <= tolerance
  }
}

sorry I can't help you much regarding this error, I was looking into rollup docs to see if I can expand debug info, but apparently this needs to be added. perhaps I'm missing something with rollup? https://github.com/themre/leaflet.pm/blob/feature/rollup/rollup.config.js

[DenisCarriere]

Humm.. 🤔 Maybe it's failing because it's a Class? Rollup won't transpile code into ES5 and Uglify needs to have ES5 code which means ES6 classes will break uglify.

[themre]

I've opened an issue with rollup uglify for someone to check if the issue is with the syntax or somewhere else. Thanks!

[DenisCarriere]

@themre You might want to have a look at rollup-plugin-uglify-es, it won't transpile to ES5, but at least it will run...

[themre]

awesome job you did @DenisCarriere ! bundle went from more than 500kb to 280 kb. For now I moved those 3 methods from jsts to separate file just to see how your modules behave. here is PR in progress:
#256