Skip to content

webpack fails at jsonld because of ES6 code in lib/jsonld.js #252

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

Closed
flyon opened this issue Jun 15, 2018 · 22 comments
Closed

webpack fails at jsonld because of ES6 code in lib/jsonld.js #252

flyon opened this issue Jun 15, 2018 · 22 comments

Comments

@flyon
Copy link

flyon commented Jun 15, 2018

webpack fails at jsonld because of the use of "..." in jsonld/lib/jsonld.js which is not ES5.

https://github.com/digitalbazaar/jsonld.js/blob/master/lib/jsonld.js#L1040

ERROR in ./node_modules/jsonld/lib/jsonld.js
Module parse failed: Unexpected token (1040:2)
You may need an appropriate loader to handle this file type.
| function _setDefaults(options, {
|   documentLoader = jsonld.documentLoader,
|   ...defaults
| }) {
|   if(typeof options === 'function') {
 @ ./src/utils/JSONLD.ts 3:15-32
 @ ./src/index.ts
Warning:  Use --force to continue.

Aborted due to warnings.
@davidlehn
Copy link
Member

What environment are you running in? I'm pretty sure there's more code that's not ES5. That's partly why the babeled dist/node6/ and webpacked dist/jsonld*.js exist. Can you just use those? The idea is that moving forward we will be using newer ES2015+ features (async/await usage is only a matter of time). If it's just one line that's causing trouble, then propose a patch to consider.

See also #251.

@flyon
Copy link
Author

flyon commented Jun 19, 2018

It's not entirely clear to me what node6 refers to and why I'd need to use it with webpack. But webpack is working now that I've switched to this (typescript) code:

 import * as jsonld from "jsonld/dist/node6/lib/jsonld";

@davidlehn
Copy link
Member

The dist/node6/ code in the npm distributions is basically just the main code run through babel so that it works with node v6. That dir will go away sometime after v6 end-of-life.

@tpluscode
Copy link
Contributor

I just got into the same problem but I cannot change the import as jsonld.js is a transitive dependency in my case.

It should definitely be possible to set up babel so that it bundles correctly. But how?

@tpluscode
Copy link
Contributor

Ok, I managed to find a workaround setting up babel as follows:

I added these babel-* dependencies. Seems to only work with all installed

yarn add --save-dev
   babel-plugin-transform-runtime 
   babel-core babel-loader 
   babel-preset-stage-0
   babel-preset-env

.babelrcthe need to load the plugin installed above

{
  "plugins": [
    ["transform-runtime", {
      "polyfill": false,
      "regenerator": true
    }]
  ]
}

webpack.config.js is tricky. My sources are in typescript so I added a loader rule specific to jsonld. Note that the include regex contains slashes to only match /jsonld/ and not other packages which may contain the word in their name.

module.exports = {
  module: {
    rules: [
        {
            test: /\.jsx?$/,
            loader: 'babel-loader',
            include: [
                /\/jsonld\//
            ],
            query: {
                presets: [ 'env', 'stage-0' ]
            },
        }
    ]
  }

@davidemontanari
Copy link

Got the same issue here.
I found a solution using resolve.alias to point to the webpacked one: https://webpack.js.org/configuration/resolve/#resolve-alias

resolve: {
    alias: {
        jsonld$: path.resolve(root, './node_modules/jsonld/dist/jsonld.min.js')
      }
}

@flyon
Copy link
Author

flyon commented Sep 1, 2018

Thank @davidemontanari that solved my issue too. I had webpack working but was experiencing problems in browsers that do not support ES6, now they were tripping up over the spread syntax "...".
With the alias you suggested it works fine

@davidemontanari
Copy link

Great @flyon! Glad to help! :)

@sanstream
Copy link

@davidemontanari and @flyon thanks for figuring out this patch, but to me this does not feel like a proper solution to the problem. Perhaps exposing the minified version (/dist/jsonld.min.js) instead of index.js (lib/index.js) in package.json as the main entry point would solve this.

Would be happy to test it.

@ragboyjr
Copy link

ragboyjr commented Feb 1, 2019

For anyone experiencing this issue while using CRA, I upgraded from CRA 1.x to 2.x and the issue went away.

@tobiasschweizer
Copy link

tobiasschweizer commented Jul 25, 2019

I have just run into the same problem using the karma test framework:

25 07 2019 17:45:20.255:ERROR [source-reader.karma-typescript]: Error parsing code: Unexpected token (121:40)
in .../node_modules/jsonld/lib/jsonld.js
at line 121, column 40:

... function(input, ctx, options) {
  if(arguments.len ...

So do I understand correctly that jsonld/lib/jsonld.js is not ES5 compliant? What is it then, ES2015?

I am using version 1.6.2.

The following construct (#251) solves my problem:

declare let require: any; // http://stackoverflow.com/questions/34730010/angular2-5-minute-install-bug-require-is-not-defined
const jsonld = require('jsonld/dist/jsonld.js');

https://github.com/dhlab-basel/knora-api-js-lib/blob/e768292199cc59e1145f68d8611ed7e311f5cc82/src/api/v2/ontology/ontologies-endpoint.ts#L8-L9

However, I would also prefer a clean solution as stated in #251.

@tpluscode
Copy link
Contributor

tpluscode commented Jul 25, 2019

Line 121 it is async function(input, ctx, options) so the parsing problem is most likely that.

I just looked at my setup where I also test TS code with karma. Apparently I explicitly include jsonld in babel-loader:

{
  test: /\.jsx?$/,
  loader: 'babel-loader',
  include: [
    /\/jsonld\//
  ],
  query: {
     presets: [ 'env', 'stage-0' ]
  },
}

@tpluscode
Copy link
Contributor

And I replied the exact same snippet last year 🤦‍♂.

Have you seen it or tried it @tobiasschweizer ?

@davidlehn
Copy link
Member

So do I understand correctly that jsonld/lib/jsonld.js is not ES5 compliant? What is it then, ES2015?

jsonld.js source is not ES5. It's using async/await so is ES2017+, I think. The npm package includes code built by webpack and babel. The dist/jsonld.* code uses whatever the babel preset-env target was when it was built. The dist/node6 code is for Node.js 6.x (which should be automatically used as needed). Modern Node.js can run all of this. Modern browsers can, I think, run the source as is. If you need to support common browser versions, the dist/jsonld.* files should likely work. Preferably you should just setup a bundler system like webpack+babel for your project, which will process jsonld.js just like any other library and output to whatever targets you want. That may be required if you want to build for pure ES5.

@tobiasschweizer
Copy link

@davidlehn

jsonld.js source is not ES5.

I am used to writing my source files in TypeScript and to distribute them as JavaScript ES5 npm packages (using the tsc transpiler). Wouldn't it be possible to have different distribution packages for jsonld for different environments rather than having different things in one npm package? There would still be only one source to maintain.

Would it make sense to add some information to the README about ES versions? I would be happy to create a PR for this. But I would surely need some assistance.

@tobiasschweizer
Copy link

tobiasschweizer commented Jul 26, 2019

I think you can't make any assumptions about the ES version of an npm package: https://stackoverflow.com/questions/43393303/which-ecmascript-version-do-the-npm-modules-use

I wonder if there could be a thing like differential loading (https://angular.io/guide/deployment#differential-loading)

@tobiasschweizer
Copy link

tobiasschweizer commented Jul 26, 2019

@tpluscode

And I replied the exact same snippet last year 🤦‍♂.
Have you seen it or tried it @tobiasschweizer ?

I don't use babel, I use tsc.

I don't understand how your include helps find the correct version of jsonld. Could you please explain that to me?

@tpluscode
Copy link
Contributor

I don't use babel, I use tsc.

Ah, bummer. I think I dropped karma-typescript for that reason. Instead, I set have karma-webpack and ts-loader to handle types script code.

I don't understand how your include helps find the correct version of jsonld.

I does not do that. It makes sure that babel compiles jsonld down to ES5.

In your case that would be a complete revolution in your case. Maybe you could try adding karma-babel-preprocessor to preprocess the sources of jsonld.

@tobiasschweizer
Copy link

I does not do that. It makes sure that babel compiles jsonld down to ES5.

Ok, now I understand :-)

Maybe you could try adding karma-babel-preprocessor to preprocess the sources of jsonld.

Good point, thanks.

@davidlehn
I am not sure I understand the concept here.
My understanding is as follows:

  • develop the source code in TypeScript (or a modern/recent version of ECMAScript)
  • transpile the code to JavaScript ES5
  • distribute the JavaScript ES5 code as an npm package (including the TS declaration files *.d.ts if the source is TS)

The source is kept under version control on GitHub, the transpiled code is distributed as an npm package (derived from source).

Now it would probably make more sense to choose ES2015 as a target since modern browsers support it. I have also read about the concept of differential loading (#252 (comment)).

@tpluscode
Copy link
Contributor

tpluscode commented Aug 4, 2019

@tobiasschweizer I just switched my project's tests from karma to jest. unless you are herd-pressed on browser testing, I highly recommend this changed. jest is much simpler to configure and the defaults just work

The good thing is that you may not need any changes in your test scenarios (I use the expect().toBe() assertion flavour.

A PR says more than a 55 words: hypermedia-app/Alcaeus#123

@tobiasschweizer
Copy link

unless you are herd-pressed on browser testing

I did not do the setup. But I think we cold give jest a try. Thanks for the hint.

@dlongley
Copy link
Member

Closing as a duplicate of #399.

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

8 participants