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

Use ES6 modules #790

Closed
sanfilippopablo opened this issue Jun 14, 2017 · 9 comments
Closed

Use ES6 modules #790

sanfilippopablo opened this issue Jun 14, 2017 · 9 comments
Assignees
Milestone

Comments

@sanfilippopablo
Copy link

Hello. Awesome project. Is there a possibility of exporting ES6 modules? This would allow to install just the main turf dependency and then let tree shaking remove everything unused.

@erikh2000
Copy link

Could you get the same benefit by using an ES6 import statement declaring specific functions your modules will use? E.g. import { difference } from '@turf/turf'

@sanfilippopablo
Copy link
Author

Full disclosure: I'm saying this by reading the source of the turf package, I haven't actually tested it.

So, currently the docs recommend to install each package corresponding to the function you use. For example, npm install @turf/sector and then import sector from '@turf/sector'. if you use multiple packages, you have to chose either:

  • Install every package you use separately and require it like that.
  • Install @turf/turf and the do exactly what you say: import { difference } from '@turf/turf'.

The problem with that second option is that the bundler will include in the bundler every function, because the turf module includes every function, even if you don't import it in any module. That's like this because the turf package index.js requires every function.

However, if that file instead of exporting like this:

isoline: require('@turf/isoline')

exports like this:

import isoline from '@turf/isoline'

export isoline

the exported modules would be ES6 modules. That means that bundlers that support tree shaking (webpack and rollup) could remove unused code, even if you install the main turf package.

@erikh2000
Copy link

Good explanation, thank you. I was hoping webpack tree-shaking would figure out the minimal dependencies with the current 'require' declarations. But I guess it doesn't from what you say. I'm no expert on webpack, myself.

@DenisCarriere
Copy link
Member

DenisCarriere commented Jun 15, 2017

We could use ES6 modules ONLY in packages/turf, we're already doing this in the Typescript definition (turf/index.d.ts).

As for all the TurfJS modules, they should remain in ES5 since NodeJS still does not natively support import & export and we should continue to support NodeJS 4 as a minimum (without any compiler).

I'm 👍 if @turf/turf would have two entry points in the package.json:

  • "main": "index.js" => Browserify build
  • "module": "module.js" => ES6 - copy paste from Typescript definition

CC: @morganherlocker

Extra references on this topic: #583 #576 #303

@sanfilippopablo
Copy link
Author

Exactly. That's at least what it says in the Rollup docs. I thought webpack also supported it, but there's an issue with a back and forth about this. Give me a day and I'll test that pull request you created against rollup and webpack to see if it works.

@DenisCarriere
Copy link
Member

👍 Sounds like a plan.

@sanfilippopablo
Copy link
Author

I'm sorry, I've been really busy, and didn't have the time to do it. Hopefully this week I'll test it.

@DenisCarriere
Copy link
Member

No worries, adding 'module' sounds like the best solution.

@stebogit
Copy link
Collaborator

Implemented with PR #793.
@sanfilippopablo feel free to open this or a new issue if your tests suggest so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants