jspm / registry

The jspm registry and package.json override service
https://jspm.io
229 stars 255 forks source link

dot-prop 4.0.0 is written in ES2015 #985

Closed oligot closed 7 years ago

oligot commented 7 years ago

Starting with version 4.0.0, the library dot-prop is now written in ES2015, which means it doesn't work in IE anymore or when minifying the code with uglifyjs as the code isn't transpiled. This commit set es6 as format so that the code is transpiled and works in IE and when minifying with uglifyjs

guybedford commented 7 years ago

Sorry just looking at this again since it is CommonJS, how is this even working with this override? Are you sure you have tested this properly?

oligot commented 7 years ago

Yes, I've tested it. Here is a small project with the override in the package.json file: https://github.com/oligot/jspm-override-dot-prop. If you remove the override in the package.json file, reinstall the dependencies (rm -rf jspm_packages && jspm install) and try to build with npm run build, there is an error in uglifyjs (probably due to the fact that uglifyjs doesn't understand ES2015 yet)

Error: SyntaxError: Unexpected token name «i», expected punc «;» (line: 38, col: 11, pos: 5174)

     Error
         at new JS_Parse_Error (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:1545:18)
         at js_error (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:1553:11)
         at croak (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:2092:9)
         at token_error (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:2100:9)
         at expect_token (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:2113:9)
         at expect (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:2116:36)
         at regular_for (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:2357:9)
         at for_ (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:2353:16)
         at eval (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:2232:24)
         at eval (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:2139:24)
         at block_ (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:2432:20)
         at eval (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:2404:25)
         at function_ (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:2410:15)
         at eval (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:2235:24)
         at eval (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:2139:24)
         at block_ (eval at <anonymous> (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/uglify-js/tools/node.js:28:1), <anonymous>:2432:20)
         at minify (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/lib/output.js:79:11)
         at exports.writeOutputs (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/lib/output.js:153:14)
         at /home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/lib/builder.js:805:14
         at tryCatcher (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/node_modules/bluebird/js/release/util.js:16:23)
         at Promise._settlePromiseFromHandler (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/node_modules/bluebird/js/release/promise.js:510:31)
         at Promise._settlePromise (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/node_modules/bluebird/js/release/promise.js:567:18)
         at Promise._settlePromise0 (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/node_modules/bluebird/js/release/promise.js:612:10)
         at Promise._settlePromises (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/node_modules/bluebird/js/release/promise.js:691:18)
         at Promise._fulfill (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/node_modules/bluebird/js/release/promise.js:636:18)
         at Promise._resolveCallback (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/node_modules/bluebird/js/release/promise.js:431:57)
         at Promise._settlePromiseFromHandler (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/node_modules/bluebird/js/release/promise.js:522:17)
         at Promise._settlePromise (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/node_modules/bluebird/js/release/promise.js:567:18)
         at Promise._settlePromise0 (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/node_modules/bluebird/js/release/promise.js:612:10)
         at Promise._settlePromises (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/node_modules/bluebird/js/release/promise.js:691:18)
         at Promise._fulfill (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/node_modules/bluebird/js/release/promise.js:636:18)
         at Promise._resolveCallback (/home/oli/src/jspm-override-dot-prop/node_modules/jspm/node_modules/systemjs-builder/node_modules/bluebird/js/release/promise.js:431:57)
guybedford commented 7 years ago

I get if it builds, but I mean does the code run? If you disable uglify does it work correctly?

oligot commented 7 years ago

Indeed, it doesn't work. Here is the error I get in the console

Uncaught (in promise) Error: (SystemJS) require is not defined
    ReferenceError: require is not defined

So, it seems like with "format": "esm", we can only use ES6 module syntax (import/export). Is there a way to configure a package so that it is recognized as an ES6 package (meaning, it will be transpiled with Babel) BUT is uses Node.js require instead of import to load dependencies ?

guybedford commented 7 years ago

Exactly. This use case can be described by setting format: 'cjs' but then still configuring the Babel loader. But this would basically mean:

{
  "format": "cjs",
  "meta": {
    "*.js": {
      "loader": "babel"
    }
  }
}

The only problem with the above is it then makes the Babel loader a dependency of the project - there is no way in jspm currently to describe that the code uses the latest ES features without tying it to a transpiler instance. I think the edge cases of transpilation are still a little bit too inconsistent for that sort of conceptual model unfortunately.

oligot commented 7 years ago

I just tried it but it doesn't work (I'm still having this error when building): https://github.com/oligot/jspm-override-dot-prop/commit/173dcd9aac20b08f005a71cf6ccd6e6ae6b00133. Do you have an idea why it doesn't work ?

guybedford commented 7 years ago

The above configuration is in jspm 0.17 only.

oligot commented 7 years ago

I've updated the project to jspm 0.17 but it still does not work.

system.src.js:122 Uncaught (in promise) Error: (SystemJS) require is not a function
    TypeError: require is not a function
        at execute (http://localhost:2000/jspm_packages/npm/dot-prop@4.0.0/index.js!transpiled:30:12)
    Error loading http://localhost:2000/app.js
        at execute (http://localhost:2000/jspm_packages/npm/dot-prop@4.0.0/index.js!transpiled:30:12)
    Error loading http://localhost:2000/app.js
guybedford commented 7 years ago

Perhaps try setting format: 'cjs' as well explicitly in the override.

oligot commented 7 years ago

It's already set

guybedford commented 7 years ago

@oligot it seems this is a bug. Try instead:

{
  "meta": {
    "*.js": {
      "loader": "plugin-babel",
      "format": "cjs"
    }
  }
}
oligot commented 7 years ago

It works now, thanks ! Note that I also had to put "modularRuntime": false in jspm.config.js I'll update the PR accordingly.

oligot commented 7 years ago

Done

guybedford commented 7 years ago

Glad to hear it is working. As mentioned previously though this isn't suitable for an override because it carries too many assumptions to provide to everyone. I would suggest keeping this as a local override in your project rather.

oligot commented 7 years ago

Ok, let's close the PR then.