mfogel / polygon-clipping

Apply boolean polygon clipping operations (union, intersection, difference, xor) to your Polygons & MultiPolygons.
MIT License
543 stars 63 forks source link

More treeshaking issues #74

Open rowanwins opened 5 years ago

rowanwins commented 5 years ago

So even after upgrading to splaytree v3 this library is failing to treeshake.

I've started a branch over here however I haven't quite got all the tests working. This commit I think captures most of the culprits, and the suggested changes were just one way to tackle them, although there may be others....

mfogel commented 5 years ago

Roger. Is there a way I can replicate the tree shaking you're doing on my end locally?

mfogel commented 5 years ago

So after reading up a bit on tree-shaking... some of the modules in polygon-clipping have minor side effects that occur on import, they don't actually affect anything outside the module but the tree-shaker may have no way to know that. Do you think that's causing the issues you're seeing?

rowanwins commented 5 years ago

Hi @mfogel

Sorry for the slow reply on this one.

My testing setup was a little less direct as it involves turf, basically I was doing something like

// index.js
import { circle } from 'turf'
var center = [-75.343, 39.984];
var radius = 5;
var options = {steps: 10, units: 'kilometers', properties: {foo: 'bar'}};
circle(center, radius, options);

and then having a rollup.config.js

import resolve from 'rollup-plugin-node-resolve';
import commonjs from 'rollup-plugin-commonjs';

module.exports = {
  input: 'index.js',
  output: {
    file: 'bundle.js',
    format: 'cjs'
  },
  plugins: [
   resolve(),
   commonjs()
  ]
};

In the bundled code I shouldn't see polygon-clipping appear but with the latest release it does. To test locally I had npm link'd polygon-clipping as a dependency for turf, and so I can make changes locally to polygon-clipping and test the bundling impacts.

In regards to the issues your comments above make sense from what I understand of tree-shaking.