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

@elastic/apm-rum-core incompatible with IE11 #377

Closed
danieljuhl opened this issue Aug 14, 2019 · 6 comments
Closed

@elastic/apm-rum-core incompatible with IE11 #377

danieljuhl opened this issue Aug 14, 2019 · 6 comments
Labels

Comments

@danieljuhl
Copy link

At line 164 in dist/lib/common/utils.js the build is using default parameters, which is not working in IE11.

function getServerTimingInfo(serverTimingEntries = []) 

When this @elastic/apm-rum is included in full bundle/webapp, the whole app/bundle is broken on IE11.

@danieljuhl
Copy link
Author

Added a month ago in c743f70

@danieljuhl
Copy link
Author

danieljuhl commented Aug 14, 2019

Integration tests was actually failing, after the change was introduced, but it was merged anyway. Not sure if the failing was related to the issues mentioned though.

@vigneshshanmugam
Copy link
Member

vigneshshanmugam commented Aug 16, 2019

We have been using default paramaters even before that PR - check this one https://github.com/elastic/apm-agent-rum-js/pull/241/files#diff-1c7652a0f8ca520a78a86b9dfcadc042R124

We publish our modules in 2 ways.

  1. Whole bundle which is transpiled with target: ie11 and could be consumed via directly. https://www.elastic.co/guide/en/apm/agent/js-base/current/getting-started.html#using-script-tags

  2. We publish the modules in both CommonJS and ES2015 modules to be consumed by both users of node and bundlers through main and module fields in package.json.

If users are going with the 2nd way which would be the preferred way in most situations, users are in full control of transpiling the code and supporting the browser version of their choice. There are multiple advantages to this

  • Bundle size can be tweaked by doing Tree shaking and dead code elimination by only importing the modules that are required.
  • users can also control the environments they are supporting by running the code through @babel/preset-env and creating separate bundles.

One of the best summary on this front - parcel-bundler/parcel#559 (comment)

If you run in to any issues or need help Let us know. We will document this from our side to make it clear.

Integration test failure is unrelated.

@danieljuhl
Copy link
Author

@vigneshshanmugam it looks like you are right, at least for some of it. I haven't analyzed everything, but it looks like the change to apm-rum between 4.0.2 and 4.1.1 (probably 4.1.0?) made our app break. Does it match the mentioned (in #383) change on how apm-rum is build? Then that's probably the reason. We haven't had issues with any other npm packages in our app.

@danieljuhl
Copy link
Author

danieljuhl commented Aug 17, 2019

@vigneshshanmugam after explicit changing exclude pattern for babel in webpack (which is normally node_modules) it works again. As it still looks like there was a change in @elastic/apm-rum / @elastic/apm-rum-core, I would suggest to at least mention this change, and one could argue that such a change to the build should have triggered a major release, as it was a breaking change, which in our case, actually broke the whole of our app, not just the apm integration.

@vigneshshanmugam
Copy link
Member

Apologies for the delay, we are working on a PR to fix this problem and users shouldnt work about running @elastic/* through babel loader. You can check this issue #426

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

2 participants