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

Untranspiled es6 distibution is breaking in IE #52

Closed
jpgorman opened this issue Dec 1, 2017 · 12 comments
Closed

Untranspiled es6 distibution is breaking in IE #52

jpgorman opened this issue Dec 1, 2017 · 12 comments

Comments

@jpgorman
Copy link

jpgorman commented Dec 1, 2017

When using the latest version of rambda, it breaks in IE as the code is untranspiled es6.

@rmdort
Copy link
Contributor

rmdort commented Dec 2, 2017

@selfrefactor Can you transpile es6 modules using babel in modules directory when you do a npm release ?

@selfrefactor
Copy link
Owner

This is from the changelog:

1.0.0 Major change as build is now ES6 not ES5 compatible (Related to issue #46)| Making Rambda fully tree-shakeable
0.9.8 Revert to ES5 compatible build - issue #46

Which means that you should use version 0.9.8, if you need ES5 comparability.

I will create another ES5 version, once there are enough changes made since 0.9.8

The current Rambda version and version 0.9.8 don't have major functional changes, so for now I suggest leave it as it is.

There is Typescript issue related to compose so when and if I find a solution, that would be big enough change to justify new es5 build.

If that doesn't happen, we'll have to wait for few more changes to come, before making es5 compatible build.

@rmdort
Copy link
Contributor

rmdort commented Dec 2, 2017

We are using rambda by importing individual modules using

import omit from 'rambda/modules/omit'

But the modules are in es6. Even 0.9.8, the files inside modules are not transpiled to es5.

For eg:

import curryThree from './internal/curryThree'

function adjust (fn, index, arr) {
  const clone = arr.concat()

  return clone.map((val, key) => {
    if (key === index) {
      return fn(arr[ index ])
    }

    return val
  })
}

export default curryThree(adjust)

The above is es6 code. Basically what i am saying is, can u publish es5 code in modules/* to npm.

@selfrefactor
Copy link
Owner

selfrefactor commented Dec 2, 2017

@rmdort I appreciate your PRs. I will keep the issue open until you and @jpgorman confirm that it is closable.

Now to add ES5 method you need to do that

const add = require('./lib/add').default

which I am not very sure that it is the desirable output.

@rmdort
Copy link
Contributor

rmdort commented Dec 2, 2017

Thanks for the merge. Now we can import modules individually without having to include entire rambda file

import omit from 'rambda/lib/omit'

And on rollup, tree shaking will work as usual with es6 imports

@jpgorman
Copy link
Author

jpgorman commented Dec 2, 2017

Rather than release two separate packages, would it be better to have an entry point for the non-transpiled es2015 code and another that works across browsers.

https://github.com/rollup/rollup/wiki/pkg.module
jsforum/jsforum#5

@jpgorman
Copy link
Author

jpgorman commented Dec 2, 2017

I suppose my point is that as a library consumer I don't want to have to transpile code from an npm module before I can use it.

@selfrefactor
Copy link
Owner

There is issue with Webpack https://stackoverflow.com/questions/46642791/webpack-doesnt-respect-module-field-in-package-json that makes your suggestion problematic.

Otherwise, I agree with you that you, as a library consumer, don't need to transpile code so you can use it.
So we are in a pickle here, and as I initially mentioned using 0.9.8 could be one way to go.

@jpgorman
Copy link
Author

jpgorman commented Dec 5, 2017

Would it instead be an option to expose a "browser" entry in the package.json and then consumers would have an option when compiling with babel or webpack etc...?

@selfrefactor
Copy link
Owner

The bug is exactly that when browser field is set, Webpack don't use esm and therefore treeshaking is useless.

New version of Webpack is already in alpha and should be a stable release soon. If the new version overcome the bug, then I would expose browser as you suggest. Otherwise, if the bug stay, then the current solution should also remain(which is to not expose browser field).

@jpgorman
Copy link
Author

jpgorman commented Dec 5, 2017

Okay, that makes sense. Thanks for the update. Is it then worth keeping this issue open until the main package is browser safe, or specify in the Docs that there is a browser safe version tag available separately?

@selfrefactor
Copy link
Owner

I will do both. Thank for the input.

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

No branches or pull requests

3 participants