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

pkg.main should be an es5 module not es6 #10849

Closed

Conversation

iainbeeston
Copy link

BACKGROUND

I'm using foundation-sites 6.4.3 in a ruby on rails app with webpacker (ie. webpack and babel), and importing the javascript code into my webpacker pack via an es6 import (ie. import 'foundation-sites'). This works great in development, however, when I try to compile my javascript for production, I get a mysterious error:

Compiling…
Compilation failed:
Hash: 3186013d0312fc90dcdc
Version: webpack 3.7.1
Time: 9176ms
                                       Asset       Size  Chunks                    Chunk Names
         application-cdc1d7b721e5671d7a43.js     891 kB       0  [emitted]  [big]  application
    server_rendering-f981a6df1b09983ee9de.js     196 kB       1  [emitted]         server_rendering
     application-cdc1d7b721e5671d7a43.js.map     169 kB       0  [emitted]         application
server_rendering-f981a6df1b09983ee9de.js.map     234 kB       1  [emitted]         server_rendering
                               manifest.json  302 bytes          [emitted]         
                            manifest.json.gz  142 bytes          [emitted]         
 server_rendering-f981a6df1b09983ee9de.js.gz    57.6 kB          [emitted]         
      application-cdc1d7b721e5671d7a43.js.gz     230 kB          [emitted]         
  [36] ./app/javascript/components ^\.\/.*$ 212 bytes {0} {1} [built]
  [52] ./app/javascript/packs/application.js + 35 modules 308 kB {0} [built]
  [54] ./app/javascript/packs/server_rendering.js 302 bytes {1} [built]
    + 52 hidden modules

ERROR in application-cdc1d7b721e5671d7a43.js from UglifyJs
Unexpected character '`' [./node_modules/foundation-sites/js/foundation.util.core.js:24,0][application-cdc1d7b721e5671d7a43.js:15181,124]

Seems like theres another report of the issue here

INVESTIGATION

It looks like uglifier is choking on es6 syntax. However, what's strange about that is that uglifier is used to post process the code, after transpiling to es5, so it should never see that syntax. What's more, so far as I'm aware, babel doesn't normally transpile node_modules, so why am I seeing es6 syntax coming from a nodule module? 🤔

ISSUE

It looks like foundation-sites switched the main value in package.json from an es5 file to an es6 file in version 6.4.0 (see d02be4b). As far as I'm aware, main is always supposed to point to precompiled es5 file. There's some discussion about how to expose an es6 module in package.json (see jsforum/jsforum#5), with jsnext:main being the current favourite, but this should be a precompiled file with any import statements removed (again, see the previous github issue), and npm.js (which is the current main value) has import statements.

FIX

For now I've reverted the change to main so that it points to an es5 file. This resolves the compile issue for me.

This shouldn't break anythinfg for users just importing everything, but might cause issues if someone wants to do a more complex import (ie. import {foo} from 'foundation-sites`).

A longer term fix might be to add jsnext:main to package.json, and change npm.js so that it's a single precompiled file that does not include import statements.

@ncoden
Copy link
Contributor

ncoden commented Jan 10, 2018

Related to #10774

@@ -1,7 +1,7 @@
{
"name": "foundation-sites",
"version": "6.4.3",
"main": "dist/js/npm.js",
"main": "dist/js/foundation.js",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you said, reverting to dist/js/foundation.js would break a lot of builds. But moving to ES6 as default was definitely not a good move. We're in a complicated position here, with both es6 and non-es6 users expecting to be able to import Foundation with the main.

We cannot accept this PR as it is, but I'll investigate around jsnext:main.
Thank you for your contribution 👍

In your case, what you can do it to import foundation-sites/dist/js/foundation
See: https://github.com/zurb/foundation-sites/pull/10782/files

@DanielRuf
Copy link
Contributor

Doesn't have the foundation sites template have transpiling?

Generally you might want to use uglify-es.

@ncoden
Copy link
Contributor

ncoden commented Jan 11, 2018

@DanielRuf Not everyone use Foundation templates. You may want to import Foundation in a custom es5 project, without having to configure transpilation by yourself.

@ncoden
Copy link
Contributor

ncoden commented Jan 17, 2018

See #10864

@ncoden ncoden closed this Jan 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants