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

issue using in a create-react-app app #4

Closed
mvolkmann opened this issue Aug 19, 2018 · 9 comments
Closed

issue using in a create-react-app app #4

mvolkmann opened this issue Aug 19, 2018 · 9 comments

Comments

@mvolkmann
Copy link

I am using your wonderful library in a library I created. See https://github.com/mvolkmann/web-translate. Thank you so much!

When I run npm run build in an app created with create-react-app, I get the following error:

Creating an optimized production build...Failed to compile.
Failed to minify the code from this file:

        ./node_modules/translate/translate.min.js:1:6121

Read more here: http://bit.ly/2tRViJ9

I believe this happens because there is something missing in your .babelrc file.
You have this:

{
  "presets": ["env"]
}

The following may fix it because it will produce transpiled code that doesn't use async/await which is needed to run in some browsers.

  "presets": [
    [
      "env",
      {
        "targets": {
          "browsers": ["last 2 versions"]
        }
      }
    ]
  ]
@franciscop
Copy link
Owner

Could you please try this and let me know where it fails?

import translate from 'translate/translate.js';

So that the specific bit is clear; there seems to be no async/await in that file nor in the minified version (using normal promises). I suspect it comes from const though.

@franciscop
Copy link
Owner

That said, writing "browsers": ["last 2 versions"] is an arbitrary choice that I think should be taken at dev-level, not at lib-level. I should not be choosing what browsers you support, I write modern ES7+ code and you can choose what browsers to support. Doesn't that make more sense?

@mvolkmann
Copy link
Author

Including import translate from 'translate/translate.js'; in my app doesn't change anything. The error is identified as being at line 1, character 6121 in the file ./node_modules/translate/translate.min.js. That character is a semicolon at the end of a long statement that begins with var iso2={. Of course this is minimized code. What follows that is just another var statement. So I don't see anything related to ES6 or above. Very strange.

@mvolkmann
Copy link
Author

I agree with the idea that you should not have to decide what browser versions a user of your library wants to support. But looking at the web page referenced in the error message (http://bit.ly/2tRViJ9) it doesn't seem that there are any easy solutions for me. It says the following:


Some third-party packages don't compile their code to ES5 before publishing to npm. This often causes problems in the ecosystem because neither browsers (except for most modern versions) nor some tools currently support all ES6 features. We recommend to publish code on npm as ES5 at least for a few more years.

To resolve this:

  1. Open an issue on the dependency's issue tracker and ask that the package be published pre-compiled.
    Note: Create React App can consume both CommonJS and ES modules. For Node.js compatibility, it is recommended that the main entry point is CommonJS. However, they can optionally provide an ES module entry point with the module field in package.json. Note that even if a library provides an ES Modules version, it should still precompile other ES6 features to ES5 if it intends to support older browsers.
  2. Fork the package and publish a corrected version yourself.
  3. If the dependency is small enough, copy it to your src/ folder and treat it as application code.

I'm on option 1 now. ;-)
I can try options 2 and 3, but those cut me off from future improvements that you might make to your library.

@mvolkmann
Copy link
Author

mvolkmann commented Aug 19, 2018

A little more info ... I now suspect the issue is that the generated file translate.min.js that your library builds uses template literal strings (with the backticks). It seems that create-react-app's npm run build cannot handle those being in dependencies when it tries to bundle/minimize an application.

It's not clear to me what I can do to cause an npm install of your library to produce a translate.min.js file that doesn't use template literals.

I see it is Rollup that creates this file, not Babel. It appears to me that you don't configure Rollup to use Babel for any transpiling which allows the resulting file to use ES6 features. I suspect that is going to limit the number of apps that can use your library. The recommendation from the React team is that libraries should deploy a version of the code that only uses ES5 to make it compatible with more apps.

@franciscop
Copy link
Owner

franciscop commented Aug 21, 2018

I am sorry, but after much thought about this, my previous answer and reading this excellent answer by Sindre Sorhus I have decided not to support ES5. Please feel free to do Sindre's 4th recommendation:

Please do open an issue on Webpack and let them know they're causing pain for both Webpack users and Node.js module maintainers.

Since rollup results in smaller files and supporting n-2 browsers in time now is arbitrary and meaningless for the future, I'll keep releasing my libraries under modern Javascript.

EDIT: I am using .babelrc for testing.

@franciscop
Copy link
Owner

Also note, options 2 and 3 might be tricky to do by yourself since I'm doing a conditional require() in a project that is bundled through the import s. This is the best way I found to be able to do fetch() in both a browser context and a server context.

@mvolkmann
Copy link
Author

The Sindre Sorhus issue seems to have been about an issue with “require”, not whether he was going to support ES5. Are you expecting Webpack to handle transpiling code under node_modules from ES6+ to ES5 when it bundles apps that use your library? Unless I’m misunderstanding something, if you choose to not release an ES5 version of your library, a large number of web apps will not be able to use it.

@franciscop
Copy link
Owner

Exactly, Webpack transpiling ES6+ to ES5 (or whatever you specify like last 2 versions) is the right thing to do as I explained.

create-react-app is going to support exactly this at some point in the future: facebook/create-react-app#3776

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

2 participants