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

What about IE11 support? #902

Closed
1 of 2 tasks
xirzec opened this issue Mar 25, 2020 · 11 comments · Fixed by #1368 or Azure/azure-sdk-for-js#10393
Closed
1 of 2 tasks

What about IE11 support? #902

xirzec opened this issue Mar 25, 2020 · 11 comments · Fixed by #1368 or Azure/azure-sdk-for-js#10393

Comments

@xirzec
Copy link

xirzec commented Mar 25, 2020

  • This only affects the JavaScript OpenTelemetry library
  • This may affect other libraries, but I would like to get opinions here first

The product I work on (Azure SDK) has some client libraries that want to support IE11 because they have customers that bundle for the browser and IE11 is still in their supported matrix.

Right now OT won't work as built because it is compiled with target: "es2017" rather than target: "es5"

I would love to be in a world without the need for IE11 support, but it feels bad for something as low-level as tracing to not have any support story for browsers that are still officially supported by an in-market OS.

@pauldraper
Copy link
Contributor

pauldraper commented Mar 25, 2020

FYI there are usually ways for toolchains to transpile node_modules (e.g. override webpack exclude).

That said, it is common for packages to publish multiple ES versions in the same package, with "es5", "es2016", etc. keys in package.json.

@xirzec
Copy link
Author

xirzec commented Mar 25, 2020

FYI there are usually ways for toolchains to transpile node_modules (e.g. override webpack exclude).

How does one achieve this with TypeScript? I'm not sure of how to configure TS to transpile dependencies, at least not when they are shipping JS rather than TS sources. Maybe we could try to do it as part of rollup for our bundle output, but I don't think that would help our ESM output (which comes directly from TS for ease of tree-shaking.)

@dyladan
Copy link
Member

dyladan commented Mar 25, 2020

@xirzec I think paul was suggesting that we as opentelemetry publish with multiple targets so that the projects which rely on us can bundle the correct target.

@xirzec
Copy link
Author

xirzec commented Mar 26, 2020

@xirzec I think paul was suggesting that we as opentelemetry publish with multiple targets so that the projects which rely on us can bundle the correct target.

That would work! ❤️

@Flarna
Copy link
Member

Flarna commented Apr 6, 2020

see #593 for background and differences in generated async code.

@johnbigley
Copy link

Is support for ES5 planned?

@xirzec
Copy link
Author

xirzec commented Jul 29, 2020

To bump this thread again, I think for the purposes of how my team uses this, it would help greatly if @opentelemetry/api was compiled down to ES5 since that's our only runtime dependency. That shouldn't cause any big bloat from async since it's only the noop implementations. It's fine if all the actual implementations still continue to target es2017.

@dyladan
Copy link
Member

dyladan commented Jul 29, 2020

To bump this thread again, I think for the purposes of how my team uses this, it would help greatly if @opentelemetry/api was compiled down to ES5 since that's our only runtime dependency. That shouldn't cause any big bloat from async since it's only the noop implementations. It's fine if all the actual implementations still continue to target es2017.

This seems like a good tradeoff to me. There is no "real behavior" in the API to worry about anyway. The only thing I would be concerned about would be the global handling.

I think compiling the API down to the lowest possible target is also a good idea because if we want any modules like db drivers or http clients to build in support for otel, they will not be able to support IE11 if the API at least does not.

@dyladan
Copy link
Member

dyladan commented Jul 29, 2020

Would like to get the opinion of @Flarna on that. There is no use of the await keyword in the API package, which avoids the polyfill that he pointed out in #593. I built it locally with to ES5, and here are the differences I have noticed

  • No use of class keyword
  • Use of var instead of const and let
  • No use of arrow functions

Use of Symbol and global appears to be unchanged.

@markwolff
Copy link
Member

@dyladan I've stubbed something out here #1368 to begin with

@Flarna
Copy link
Member

Flarna commented Jul 29, 2020

We used transpile to ES5 for quite a while without problems. If await or generators are used polyfills are generated which may have visible impact to e.g. async hooks but this is not applicable in the packages we talk about.

I took a look into the api and context-base package and it seems the difference is small:

  • let/const => var
  • class => function
  • arrow functions to normal functions (but this behaves correct)
  • default arguments are polyfilled
  • following polyfill is created for extends (see NoopMeter) which should not harm:
var __extends = (this && this.__extends) || (function () {
    var extendStatics = function (d, b) {
        extendStatics = Object.setPrototypeOf ||
            ({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
            function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; };
        return extendStatics(d, b);
    };
    return function (d, b) {
        extendStatics(d, b);
        function __() { this.constructor = d; }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    };
})();

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