turf.md

Okay, Thanksgiving day Turf deep dive. Let's do this.

The problem: a refactored Turf relies on tree shaking to make it possible to include subsections of the module without including the whole thing. But tree shaking isn't working.

Approach

Remove indirection

We bundle from ES6 import/export modules into a single module bundle. Instead, set module: 'src/index.js' to remove a potential problem - Rollup bundling.

Reduce

Choose a module that I believe can be clearly tree-shaked: I choose convertDistance. Then make sure it is shaken. It is not.

Remove more complexity: comment out the rest of the exports in src/index.js. Now try importing convertDistance. This works now: it is included.

Next, uncomment modules one by one, running

./node_modules/.bin/rollup -c rollup.test.js && du -sh output.js

Each time, and seeing which ones trigger bundle blowup.

At this point a theory appears: JSTS. This makes sense, because JSTS has essentially been the chaos monkey of Turf ever since Turf mentioned it: whether it's browser compatibility, bundle size, esoteric bugs - JSTS has caused many problems.

But uncommenting simplify triggers the inclusion of simplify even if it isn't included. Why? Is it CommonJS triggering this problem?

Doesn't seem like that - simplify is a vendored dependency.

Modules that trigger tree fail

• things that reference jsts
• simplify

I've now isolated all the modules that trigger tree-fail.

Now let's find the simplest possible module that trigger tree-fail. Ideally one that spikes it only from 4 to 8k.

Did some digging into Rollup. Now trying

	treeshake: {
      propertyReadSideEffects: false,
      pureExternalModules: true
}

Now noticing a super minor Rollup bug that I hadn't before: if Rollup tree-shakes all the code out of a file, it sometimes (all the time?) leaves its comments anyway. This means that the difference between 4k and 8k, which I previously thought was relevant, is not. That means a lot more modules are already working.

Findings

Okay, I have a first finding!

And Rollup is right this time! Okay, so the 'dissolve' Turf module requires (through the require chain) a module called 'qheap' via polygon-clipping, which includes this groan-worthy hack:

 if (Number.EPSILON === undefined) Number.EPSILON = Math.pow(2, -52);

This is a true side-effect and Rollup is fully correct in including qheap, even if it isn't in the imported names.

Okay, running through the rest...

Looks like geojson-rbush is a big trigger too.

line-split -> center of mass -> convex -> concaveman

concaveman -> rbush

geojson-rbush -> rbush

Theme

Basically Turf gets pwned by its CommonJS dependencies. In the case of qhull, it looks like that's because they have clear side-effects.

Correction: polygon-clipping itself includes the IE polyfill, as well as including QHeap, which also includes the IE Polyfill

Actions?

• Either PR against polygon-clipping or vendor it and remove the Number.EPSILON polyfill. I've tested that this would work if done, and there's zeeero reason to polyfill it: just assign a variable, don't modify globals.

Next up: polygonize. I think the key problem is:

Object.defineProperties( EdgeRing.prototype, prototypeAccessors );

There's no reason to do it this way: just use a loop or define them directly. Fixing this fixes the tree-fail.

Next up: clusters-dbscan. I see that it depends on density-clustering, and that module has a bower.json, which does not inspire faith. Taking a look. That module has these

if (module.exports) {
module.exports = ...
}

Sections in it, which I guess are intended to make it possible to script-tag it (as Bower wants)

Removing all of those fixes tree-shaking.

Actions:

• PR or vendor density-clustering, remove the if/else junk.

Next up: boolean-parallel. What's up with you. Distills to bearingToAzimuth import from turf/helpers. That's... wrong. It should be ../helpers.

Next up: bezier-spline. We're already vendored here, which is great. Swapping exports['default'] = Spline with export default Spline fixes it.

Next up: boolean-overlap. Reduces quickly to line-overlap.

Method note: I'm using watchman and this command:

watchman-make -p 'test.js' 'src/**/*.js' 'rollup.test.js' --run ./test.sh

And

./node_modules/.bin/rollup -c rollup.test.js && du -sh output.js

And test.js:

import { convertDistance } from "./";

console.log(convertDistance);

Back to the hunt.

Okay, per geojson-rbush, action is:

• Probably vendor geojson-rbush. It relies back to turf anyway and is tiny.

Hey @tmcw

Thanks for taking a look into this again and for the awesome documentation.

On the positive side this was pretty much the approach I was taking with my investigations, the part I hadn't yet done was looking down at those 3rd party dependencies which is where it looks like most of the work lies.

Thanks also for the tip on watchman

I'll start taking a look at those bits and start trying to progress on the fixes.

Thanks,
Rowan

So looking into this

• I've made a start on pulling in geojson-rbush into turf itself, I'm thinking perhaps I'll make it a new module called index, or perhaps spatialIndex(which may be less risky as a module name)...

• The most current version of polygon-clipping removes the qheap dependency so that's a start, although it's still doing the IE11 polyfill itself. I've been semi-involved in that project so getting a PR in there shouldn't be too hard.

  • The polygon-clipping module is also used in union, clip & difference

Modules that need fixing

• booleanEqual

• I've got an issue in with rbush to make it ES6 so we'll see how that goes. I've brought in the geojson-rbush part into turf itselfas spatial-index.