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

ES5 support is broken for Angular users (MathJS 6+) #1565

Closed
bcmedeiros opened this issue Jul 9, 2019 · 18 comments
Closed

ES5 support is broken for Angular users (MathJS 6+) #1565

bcmedeiros opened this issue Jul 9, 2019 · 18 comments

Comments

@bcmedeiros
Copy link

I have just upgraded my Angular project to use MathJS 6.0.3 so I could use tree-shaking and reduce my bundle size. The first test was ok, everything good on chrome, bundle is 1.5MB smaller (amazing work, BTW, thank you very much for that), but the app started failing on IE11.

Since I was running the production build, the first error I could extract was this one:
image

The (selected) arrow function was crashing IE.

After that, I tried to reproduce the problem with dev build and the error moved to a different place:
image

Again, IE crashes on the =>.

I confess I was a little bit lost with this issue, I could swear angular-cli would take care of the transpilation of libraries, but it seems that it doesn't. So, it looks like MathJS 6+ is now the first library that I'm using that is breaking ES5 compatibility, and since the download page states that Math.js works on any ES5 compatible JavaScript engine, I'm raising this issue.

Is this by design and MathJS 6 requires ES2015+ engines? I believe we should either change the documentation or fix these compatibility issues.

Please, let me know if you need any help.

@ericman314
Copy link
Collaborator

This could possibly be related to #1558. I'm not very familiar with the issue myself but I expect a fix is in the works.

@bcmedeiros
Copy link
Author

Eric, I don't see how those problems could be related (the bug seems to be related to a few methods while this one happens with a single arrow function), but I don't know much about babel as well, so you might have a point.

@harrysarson
Copy link
Collaborator

Yeah sorry @brunojcm this is an upstream issue with Babel and we are waiting for a fix from them.

We should indeed support ie 11

@bcmedeiros
Copy link
Author

Thanks @harrysarson! Could you please refer to the upstream issue, if you have it handy, so I can follow it?

@ericman314
Copy link
Collaborator

babel/babel#10166

@bcmedeiros
Copy link
Author

Well, this is the same issue mentioned in the PR Eric posted here, so I still don't get how an issue related to the getOwnPropertyDescriptors method can cause errors with arrow functions. I know nothing about babel, though, so I will trust you on that :)

@harrysarson
Copy link
Collaborator

We test maths on ie 11 in our ci and this issue came up when the newest version of Babel was published. I have no idea exactly what getOwnPropertyDescriptors and arrow functions have to do with each other though.

Possibly the error you are seeing in your browser has been translated by source maps?

@bcmedeiros
Copy link
Author

No luck with the PR.

Tried enabling arrow function plugin, no luck as well.

Something looks very wrong here. I can't find any npm script to pass src files through babel, I'm just copying it over, and angular cli seems to be getting the failing line, const createTrue = /* #__PURE__ */ Object(_utils_factory__WEBPACK_IMPORTED_MODULE_0__["factory"])('true', [], () => true), from ./node_modules/mathjs/src/utils/bignumber/constants.js. If this file doesn't go trhough babel somehow, I don't see how it could work on ES5.

@bcmedeiros
Copy link
Author

One thing worth mentioning is that I'm importing like

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

which seems to follow export * from './main/es6/number' in numbers.ts. Not sure if that matters, but I'm running out of options here.

@josdejong
Copy link
Owner

@brunojcm the export * from './main/es6/number' is the one you want, though this points to ES6 code containing for example arrow functions => that still needs to be transpiled to ES5. Is your Webpack setup configured such that it also transpiles ES6 code from from dependencies like mathjs?

This may be similar to #1554

@bcmedeiros
Copy link
Author

This may be similar to #1554

That's indeed very similar!

@brunojcm the export * from './main/es6/number' is the one you want, though this points to ES6 code containing for example arrow functions => that still needs to be transpiled to ES5. Is your Webpack setup configured such that it also transpiles ES6 code from from dependencies like mathjs?

No, and AFAICS angular CLI does not transpile libs. I don't even know where to start, and based on angular/angular-cli#9126 it doesn't seem something easy to maintain.

I still have to study a lot more about all these JS tools/packagers, but what seems wrong to me is to copy all src and main folder into the output, as it duplicates all the code that has been translated by babel in the dist/math.js file.

@bcmedeiros
Copy link
Author

bcmedeiros commented Jul 10, 2019

Some new findings here, it seems that importing from mathjs/main/es5/number still gives me a reduced bundle size and works on IE11.

I use MathJS in two points here, one in the main app, which goes into the vendor chunk, and the other one in a lazy loaded angular module, which goes into the (let's call) lz1 chunk.

main: import { evaluate } from '___'
lazy1: import { evaluate, round } from '___'

Here are the numbers I get replacing ___ by different imports:

  1. No MathJS import (baseline)
    chunk {vendor} 6.31 MB
    chunk {lazy1} 2 MB

  2. mathjs
    chunk {vendor} 10 MB
    chunk {lazy1} 2 MB
    IE11 ❌

  3. mathjs/number
    chunk {vendor} 8.46 MB
    chunk {lazy1} 2 MB
    IE11 ❌

  4. mathjs/main/es5/number
    chunk {vendor} 7.85 MB
    chunk {lazy1} 2 MB
    IE11 ✌️ (but fails production build, see comments below)

  5. mathjs/main/es6/number
    chunk {vendor} 8.35 MB
    chunk {lazy1} 2 MB
    IE11 ❌

These numbers don't make much sense for me, but that's what I got here. Hopefully that will help you figure out what's going one. For now, I'll stick with importing from mathjs/main/es5/number as it gives me the smaller bundle and works on IE11.

@josdejong
Copy link
Owner

If you use mathjs/main/es5/number, it will use the whole library but just the number implementation, so that's much smaller already. In your case, you're using evaluate, which has a dependency on all functions, so you can't go any smaller and this solution may be just fine.

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 IE11 and other browsers, and treeshaking works as expected (only thing was I had to add a polyfill for Object.assign).

So it looks like you'll have to dig into your webpack setup further, or just use the es5 version.

@bcmedeiros
Copy link
Author

I created an empty React application using create-react-app.

I'm not sure about the differences between Angular and React, but I tried the same with both React and Angular, and for Angular and it failed again on IE 11, it's not something specific to my app.
File: vendor.js, Line: 78185, Column: 114

This is the Angular project I tested with (target set to es5):
my-app-math.zip
To run:
npm install
ng serve

So it looks like you'll have to dig into your webpack setup further

Again, I'm far from being an expert, but comments on this issue does not encourage that at all. At least for Angular CLI users changing Babel/Webpack directly is very hard, if possible. ng eject is now deprecated and there seems to be no reasonable way to achieve that.

or just use the es5 version.

Not an option anymore... I just found out that importing from mathjs/main/es5/number fails in the production build with the error Object(...) is not a function immediately after call evaluate, for both Chrome and IE11, so it seems that I will have to revert to MathJS 5 until we figure out what's going on.

@bcmedeiros bcmedeiros changed the title ES5 support is broken on MathJS 6+ ES5 support is broken for Angular users (MathJS 6+) Jul 16, 2019
@josdejong
Copy link
Owner

Thanks for sharing you project, I will try to do some debugging soon.

Having to do complicated babel/webpack setups in angular projects shouldn't be needed, it should just work for Angular projects as it does for React. Haven't used Angular in years myself though, any help would be very welcome.

@bcmedeiros
Copy link
Author

I'd love to help more, but I'm afraid I don't have that much knowledge on how to create a library with ES6 and ES5 support in parallel, and also how Angular CLI bootstraps Webpack (I don't even know if what I say makes sense in this regard haha), so I probably won't be able to help much more for now :( I'll try to have a look again once I have more free time to revisit it.

@josdejong
Copy link
Owner

Probable cause: #1554 (comment)

@josdejong
Copy link
Owner

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

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

No branches or pull requests

4 participants