Turf: Major release pre-announcement (TurfJS v5.0)

[DenisCarriere]

Major release pre-announcement (TurfJS v5.0) ⭐️ 📢

Due to some drastic changes coming up, the next TurfJS release will be v5.0 (Major release).

If anyone has any suggestions for breaking changes, the time would be now to do it.

Timeline

Approximately a 1-2 weeks (end of September/ early October, 2017).

Major To-Dos before publish

• [ ] Finish adding new optional parameters (options) (PR https://github.com/Turfjs/turf/pull/953)
• [x] Convert to ES6 modules using Rollup (PR https://github.com/Turfjs/turf/pull/960)
• [ ] [v5.0 issues](https://github.com/Turfjs/turf/labels/major-release)

Not happening for next publish

• [x] jsts won't be entirely removed

@stebogit @rowanwins Any additional suggestions for v5.0 changes?

Don't hesitate to send some breaking PR changes (within reason & approved).

CC: @Turfjs/ownership

[stebogit]

There's all this stuff waiting for the next major release:
https://github.com/Turfjs/turf/labels/major-release
They should be easy improvements. I'll start working on it as soon as I can

[rowanwins]

Aoowww I so wanted to remove jsts for v5 but I think your right, it probably won't happen unfortunately :(

[DenisCarriere]

@rowanwins wanted to remove jsts for v5

Maybe this will help solve our jsts large bundle size issue:
https://github.com/bjornharrtell/jsts/pull/317

@rowanwins Do you want to attempt to see if you can make it work with the ES6 module jsts bundle?

JSTS as a ES6 module Rollup bundle
https://gist.github.com/DenisCarriere/7b268a90f341cf9da2574a9f845b64b3

[DenisCarriere]

@stebogit There's all this stuff waiting for the next major release:

😄 👍 Awesome, I'll tackle a few of those as well.

[bjornharrtell]

JSTS - the undead legacy library ported from lovely Java 4 (!) compatible code that just... wont.. die. :smiling_imp:

[DenisCarriere]

@bjornharrtell Very 👍

[ffflabs]

I believe @turf/meta needs a default export in the form

var meta = {
    coordEach: coordEach,
    coordReduce: coordReduce,
    propEach: propEach,
    propReduce: propReduce,
    featureEach: featureEach,
    featureReduce: featureReduce,
    coordAll: coordAll,
    geomEach: geomEach,
    geomReduce: geomReduce,
    flattenEach: flattenEach,
    flattenReduce: flattenReduce,
    segmentEach: segmentEach,
    segmentReduce: segmentReduce,
    lineEach: lineEach,
    lineReduce: lineReduce
};
export default meta;

because dependencies (such as geojson-rbush) do still require it as

var meta = require('@turf/meta');

So trying to build, for example, a pure ES6 version of @turf/line-overlap (which depends on geojson-rbush) using rollup will complain:

(!) Missing exports
https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module
commonjs-proxy:<fork folder>/turf/packages/turf-meta/index.js
default is not exported by packages/turf-meta/index.js

Of course, there is still the issue about importing jsts, since jsts's package.json declares an empty index.js as its module property.

[DenisCarriere]

@amenadiel Agreed... 🤔 Seems like everything needs to be changed to Modules or add more "support" for other libraries that don't want to Modules.

We're definitely in a transition phase with CommonJS + ES Module libraries.

~I'll add export default to the libraries which have multiple methods.~ I'll update geojson-rbush to pure ES6.

I've went back and forth with this decision a lot... 😠

[DenisCarriere]

Of course, there is still the issue about importing jsts, since jsts's package.json declares an empty index.js as its module property.

Agreed! That's also an issue

[bjornharrtell]

Hmm I still can't understand why that's an issue and why you can't import individual modules from the package. See https://github.com/bjornharrtell/jsts/pull/317 (was closed with no response to my review comment).

[DenisCarriere]

why you can't import individual modules from the package.

@bjornharrtell It's an issue (on Turf's side) right now since we only import jsts at the root var jsts = require('jsts'), we have to change all the modules that depend on jsts using the new ES module syntax.

[DenisCarriere]

I believe the biggest issue is the lack of documentation on how to use ES Modules with jsts.

CommonJS

var jsts = require('jsts')
var reader = new jsts.io.WKTReader()
var a = reader.read('POINT (-20 0)')
var b = reader.read('POINT (20 0)')
a = a.buffer(40)
b = b.buffer(40)

Most common ES modules imports

import {WKTReader} from 'jsts/io'

import * as io from 'jsts/io'
const WKTReader = io.WKTReader

import WKTReader from 'jsts/io/WKTReader'
var a = reader.read('POINT (-20 0)')
var b = reader.read('POINT (20 0)')
a = a.buffer(40)
b = b.buffer(40)

@bjornharrtell None of them work, are you expecting someone to guess where these modules are located?

[bjornharrtell]

I agree some more documentation would be good. If you know the upstream (JTS) it's easier, but still. Bear in mind the ES module support is rather new and that JSTS is a one man project. I think you might be the first user actually getting to depend in it this way, except me.

I'll try to use your examples as a starting point for a ES module quick start.

For WKTReader the correct module path is jsts/org/org/locationtech/jts/io/WKTReader but as you noted there might be something wrong there...

[ffflabs]

@DenisCarriere I've been playing a bit with this based in @bjornharrtell comment about his topolis project and it seems it's not that difficult to use named imports from jsts.

Given you have jsts installed as a dependency in your node_modules folder, you can replace every occurence of

var jsts = require('jsts');

with

import {
  GeoJSONReader,
  GeoJSONWriter
} from 'jsts/org/locationtech/jts/io.js';

import 'jsts/org/locationtech/jts/monkey.js'

And every occurence of jsts.io.GeoJSONReader and jsts.io.GeoJSONWriter with just GeoJSONReader and GeoJSONWriter.

This affects modules turf-buffer,turf-difference,turf-intersect and turf-union.

You need to include monkey.js to extend Geometry.prototype with methods such as buffer, which won't be implicitly imported just from the io namespace.

PS I still had to include the polyfills in

/node_modules/jsts/org/locationtech/Array.js
/node_modules/jsts/org/locationtech/Number.js
/node_modules/jsts/org/locationtech/Math.js

in my karma.conf.js for PhantomJS to pass the tests. (I'm using turf as a dependency on another project which uses this approach...)

[DenisCarriere]

👍 Ok Thanks for your help @bjornharrtell I've made some progress using the ES modules.

It's still very cryptic.. but once we have a few examples it will be easier to implement the newer ES module syntax.

[DenisCarriere]

So far I've got buffer working with the following code:

import extend from 'jsts/extend'
import Geometry from 'jsts/org/locationtech/jts/geom/Geometry'
import WKTReader from 'jsts/org/locationtech/jts/io/WKTReader'
import {BufferOp} from 'jsts/org/locationtech/jts/operation/buffer'

extend(Geometry.prototype, {
    buffer: function () {
        return BufferOp.bufferOp(this, ...arguments);
    }
})

export default () => {
    const reader = new WKTReader()
    let a = reader.read('POINT (-20 0)')
    let b = reader.read('POINT (20 0)')
    a = a.buffer(40)
    b = b.buffer(40)
    return a
}

[DenisCarriere]

@amenadiel I wouldn't recommend using monkey.js since that will import the entire library, which would defeat the entire purpose of Rollup and it would make the Rollup bundle huge.

One of the strengths of ES modules is it allows Tree Shaking, importing monkey.js would remove that feature.

[DenisCarriere]

Here's a breakdown of the buffer example filesize:

• normal => 408KB (14,600 Lines of code)
• minified => 256KB

[DenisCarriere]

@bjornharrtell @amenadiel I've posted some working examples of jsts with ES6 modules.

https://github.com/DenisCarriere/jsts-es6-example

@amenadiel You need to include monkey.js to extend Geometry.prototype with methods such as buffer, which won't be implicitly imported just from the io namespace.

Look at the buffer example I've created, I added a custom monkey.js process to include only the buffer operation.

[ffflabs]

You're right, monkey adds a lot of overhead. However I always try to tamper as little as possible with third party code, so I can update my deps transparently.

By monkey patching the Geometry proto yourself you're introducing a coupling, given the unlikely case that jsts buffer footprint ever changes.

This is still a giant leap ahead regarding the seamless integration with jsts methods.

[ffflabs]

PS I'm all about tree shaking. However I still believe the right approach would be to submit a PR against jsts to split monkey.js into several decorators.

[bjornharrtell]

Monkey is about keeping API compatibility with previous version and upstream but are really only delegates from Geometry to other modules. Was broken out into a hacky money patch because it creates evil circular dependencies. Would like to see it removed in the future but depends on upstream.

The monkey patch can be avoided entirely, but it's not clearly documented yet (although can be found in API docs). I made this example yesterday http://bjornharrtell.github.io/jsts/index_modules.html that might shed some light...

[DenisCarriere]

@bjornharrtell 👍 Nice, I'll try to use your Index modules suggestions in the coming PR:

https://github.com/Turfjs/turf/pull/1025

var difference = OverlayOp.difference(a, b)

Looks easy enough to use this approach.

[DenisCarriere]

👍 Works like charm (no monkey patching required)

[DenisCarriere]

v5.0 is now published