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

Unexpected token in round.js #1554

Closed
mockdeep opened this issue Jul 1, 2019 · 19 comments
Closed

Unexpected token in round.js #1554

mockdeep opened this issue Jul 1, 2019 · 19 comments
Labels

Comments

@mockdeep
Copy link

mockdeep commented Jul 1, 2019

When upgrading to MathJS 6.0.2, we get an error when trying to make use of mathjs in our application, compiling with Webpack 3.4.0:

11:13:27 webpack.1           | ERROR in ./node_modules/mathjs/src/function/arithmetic/round.js
11:13:27 webpack.1           | Module parse failed: Unexpected token (58:4)
11:13:27 webpack.1           | You may need an appropriate loader to handle this file type.
11:13:27 webpack.1           | |    */
11:13:27 webpack.1           | |   const round = typed(name, {
11:13:27 webpack.1           | |     ...roundNumberSignatures,
11:13:27 webpack.1           | | 
11:13:27 webpack.1           | |     'Complex': function (x) {
11:13:27 webpack.1           |  @ ./node_modules/mathjs/src/factoriesAny.js 98:0-57
11:13:27 webpack.1           |  @ ./node_modules/mathjs/src/entry/mainAny.js
11:13:27 webpack.1           |  @ ./node_modules/mathjs/main/es6/index.js
11:13:27 webpack.1           |  @ ./app/javascript/src/config/math.ts

Here's what it looks like where we set up MathJS:

import {
  create,
  addDependencies,
  BigNumberDependencies,
  divideDependencies,
  evaluateDependencies,
  multiplyDependencies,
  subtractDependencies,
} from 'mathjs';

// http://mathjs.org/docs/custom_bundling.html

const math = create([
  addDependencies,
  BigNumberDependencies,
  divideDependencies,
  evaluateDependencies,
  multiplyDependencies,
  subtractDependencies,
]);

export default math;
@josdejong
Copy link
Owner

It looks like webpack uses the es6 source code in the folder ./main/es6, which contains ES6 modules, but does not compile this code to ES5. I think you either have to configure your webpack setup to also compile the ES6 code of mathjs. Alternatively you could use the ES5 code which is available in ./main/es5, but you would lose support for tree shaking.

@mockdeep
Copy link
Author

mockdeep commented Jul 1, 2019

Hmm, I don't think we've had this problem with any other libraries. My understanding is that libraries will generally compile down to ES5 for their released version. I guess we can use the es5 version. In mathjs 5.10.3 we have been manually importing all of the pieces we need like so:

import add from 'mathjs/lib/function/arithmetic/add';
import bignumber from 'mathjs/lib/type/bignumber';
import core from 'mathjs/core';
import divide from 'mathjs/lib/function/arithmetic/divide';
import mathEval from 'mathjs/lib/expression/function/eval';
import multiply from 'mathjs/lib/function/arithmetic/multiply';
import subtract from 'mathjs/lib/function/arithmetic/subtract';

// http://mathjs.org/docs/custom_bundling.html

const math = core.create();

// types need to be imported before functions
math.import(bignumber);

math.import(multiply);
math.import(add);
math.import(subtract);
math.import(divide);
math.import(mathEval);

export default math;

It doesn't appear that we can do it like this anymore, though, unless I'm missing something. It looks like there is a tree of dependencies that need to be handled.

@josdejong
Copy link
Owner

My understanding is that libraries will generally compile down to ES5 for their released version.

That's correct, though more and more libraries (and mathjs now too) start exposing ES6 modules, amongst others to better optimizing of bundles using tree shaking out of the box.

This should be simply a matter of configuring your Webpack setup to also transpile the mathjs code. Maybe the following topic is helpful: babel/babel-loader#171 (comment)

@josdejong
Copy link
Owner

I've just done a test to double check whether the exported modules and treeshaking works correctly. I created an empty React application using create-react-app. In there, I imported mathjs like:

import { version, evaluate } from 'mathjs/number'

When building the application, it runs just fine on all browsers including IE11, and treeshaking works as expected (only thing was I had to add a polyfill for Object.assign for IE).

@taozi926494
Copy link

@josdejong Hmm, I also create a test demo by vue-cli3 (use mathjs 6.0.3). Appear the same error in Edge and other low version browsers (e.g. QQ browser 9.6.3 [ based on chromium53.0.2785 ]). I just do this
-- mathtest.js`

import { random, evaluate, round } from 'mathjs/number';

export default () => {
    let __result = random(0, 100);
    __result = round(__result, 2);
    return __result;
};

-- App.vue

<template>
  <div id="app">
    {{ result }}
    <button @click="calc">click me</button>
  </div>
</template>

<script>
import func from '@/components/mathtest.js';

export default {
  name: 'app',
  data() {
    return {
      result: 0,
    }
  },
  methods: {
    calc() {
      this.result = func();
    }
  }
}
</script>

@josdejong
Copy link
Owner

josdejong commented Jul 28, 2019

@mockdeep the error message is about webpack not recognizing the ... spread operator. Can it be that your webpack setup doesn't support the spread operator and you need to update your setup? Webpack 3.4.0 was released two years ago (!). See also webpack/webpack#5548.

@josdejong
Copy link
Owner

@taozi926494 can you create a separate issue for the vue3 thing? I did a bit of debugging with your code, It seems to have a different cause. Not entirely sure though. Can you create a codesandbox or stackblitz example demonstrating the issue?

@mockdeep
Copy link
Author

@josdejong I think the problem is, as you mentioned, that we're not transpiling mathjs with Webpack. We've got plenty of spread operators in our own code, but we don't transpile node modules, as that would take forever. We'll leave it locked at version 5.x for now until we can figure out a better option.

@josdejong
Copy link
Owner

@mockdeep did you try upgrading Webpack? That may be a good idea anyway?

@josdejong
Copy link
Owner

I may understand the issue now: the mathjs configuration in package.js has a module file which points to non-transpiled modules. I guess mathjs should transpile the modules: import/export must stay, but const, =>, ..., etc must be transpiled. Will give that a try.

@mockdeep
Copy link
Author

did you try upgrading Webpack?

@josdejong we're currently on version 3.4.0. We haven't had a chance to upgrade to version 4.x, but I also don't think that should matter for this use case.

import/export must stay, but const, =>, ..., etc must be transpiled

I think import/export should also be transpiled to require/module.exports syntax. If you install the react npm package for example, you'll see that all of their code uses that format.

@josdejong
Copy link
Owner

I think you're right that it's not because of an old version of Webpack. I indeed think it's because the module code should be transpiled to ES5. Still have to figure out how to get that working with babel (tried the esmodules option but it's not working or I don't understand it yet). We should keep import/export though in the transpiled module code, and not replace that with commonjs, since that enables webpack and rollup to do treeshaking magic. See for example what lodash-es vs lodash.

@mockdeep
Copy link
Author

mockdeep commented Aug 1, 2019

lodash-es is the special version for ES6 tree shaking support, but lodash still compiles to using require/module.exports.

@josdejong
Copy link
Owner

Yes indeed. I would like to offer both with mathjs: the main field in package.json points to the ES5+commonjs, the module field should point to ES5+import/export (and currently the latter is not ES5, I think that's the only thing that should be fixed).

@mockdeep
Copy link
Author

mockdeep commented Aug 2, 2019

Ah, okay, I see what you're saying now. I'm a little surprising that the ES modules one needs to be transpiled to ES5 still.

@mockdeep
Copy link
Author

mockdeep commented Aug 4, 2019

Okay, I did some digging and did my best to answer your question.

@josdejong
Copy link
Owner

That's it indeed 🎉, thanks a lot. I have a working proof of concept now. I hope to work it out tomorrow night and publish the fix.

@josdejong
Copy link
Owner

This issue should be fixed now in v6.0.4, can you give it a try @mockdeep?

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

No branches or pull requests

3 participants