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

Investigate removing the globals __extends() and __assign() populated by applicationinsights-shims #1280

Closed
MSNev opened this issue Jun 3, 2020 · 19 comments
Assignees
Milestone

Comments

@MSNev
Copy link
Collaborator

MSNev commented Jun 3, 2020

As part of removing tslib usage because of #1269, we created the applicationinsights-shims package, which to ensure that any modules that uses it also exposes the __extends() and __assign() onto the global scope (it reuses any existing instances).

We have had a request that at least for the ESM modules (further details will be provided) if we can not pollute the global scope with these functions.

@markwolff
Copy link
Contributor

Seems like a good time to rip off the band aid and bump to typescript 3. WDYT?

@MSNev
Copy link
Collaborator Author

MSNev commented Jun 3, 2020

It's not that easy (as I already tried that before breaking the dependency), the issue is that we are targeting es3 and the "PropertyKey" added by tslib is not available when compiling for a es3 target.

I've not tried using v2 of tslib with TS 3.x yet though, but I expect the same issue (if not more) -- one of the reasons I called this "Investigate".

And we also need to make sure that the generated code / modules don't break older users with that sort of move :-(

@CraigComeOnThisNameCantBeTaken

I am using Angular universal along with this package, and __extends and __assign are both undefined for me.

@kryalama
Copy link
Contributor

@CraigComeOnThisNameCantBeTaken, can you please share the details of the "@microsoft/applicationinsights-web" version you are using in your code. We want to make sure "@microsoft/applicationinsights-shims": "^1.0.0" in your dependency list. Please refer to suggestions from #1281.

@CraigComeOnThisNameCantBeTaken
Copy link

@CraigComeOnThisNameCantBeTaken, can you please share the details of the "@microsoft/applicationinsights-web" version you are using in your code. We want to make sure "@microsoft/applicationinsights-shims": "^1.0.0" in your dependency list. Please refer to suggestions from #1281.

Just to add to this, I found out domino was in use in the project which polyfills the window object and which seems to be the cause of my issues. With domino some logic must fall over in the shim...

Anyway this is my fault - the Angular framework already gives tools for cross-platform running and so polyfilling with domino makes no sense, however I guess it highlights the unexpected problems you can have with the global scope. I had to do some reverse engineering of my built app to pinpoint the problem.

@MeghaY
Copy link

MeghaY commented Aug 21, 2020

@kryalama : The exact same issue is occurring with me. I am seeing __assign() not defined error. I tried to follow issue #1281 and added the Plugin in webpack as described in #1281 but still facing the same issue.

@MSNev
Copy link
Collaborator Author

MSNev commented Aug 27, 2020

Did you use the applicationinsights-shims or the tslib version?

I have found (using a different modules) that Webpack ProviderPlugin can silently fail (and not include the references) if the module can't be found for some reason, which results in the same errors.

When the packages are correctly referenced and available the ProviderPlugin does work.

@MLoughry
Copy link

Perhaps I'm naïve, but what browsers are you trying to support by targeting es3 instead of es5?

@MSNev
Copy link
Collaborator Author

MSNev commented Aug 31, 2020

Primarily IE8, we have customers that still need to target IE8 and a couple in the health industry that still need IE7.

@MLoughry
Copy link

Could you have a separate build under lib-esm/ or similar that targets es5 and esnext modules, then specify the path as "module": in the package.json? This would be a backwards-compatible change that would allow apps targeting modern browsers to both optimize/tree-shake the package bundle themselves, and avoid the tslib workaround.

@MSNev
Copy link
Collaborator Author

MSNev commented Sep 2, 2020

The problem with that is many fold, however, this work-around actually has nothing to do with ES3 and our support for IE8.

The issue is one of compilation, we can't compile with the currently targeted version of TypeScript (2.5.3) AND use version 1.13.0 (or later) of tslib -- as fails to compile. The workaround was to break the dependency so that we don't break other teams / customers by targeting the older versions of tslib.

In short, until we can successfully upgrade the version of typescript that we use (work in progress) without breaking other teams or customers we are limited to using this workaround... Unless of course if tslib publishes a fix.

MSNev added a commit that referenced this issue Apr 2, 2021
…les that include the shims module from the all AI modules so that webpack can evaluate correctly #1509

Investigate removing the globals __extends() and __assign() populated by applicationinsights-shims #1280
MSNev added a commit that referenced this issue Apr 2, 2021
…les that include the shims module from the all AI modules so that webpack can evaluate correctly #1509

Investigate removing the globals __extends() and __assign() populated by applicationinsights-shims #1280
MSNev added a commit that referenced this issue Apr 3, 2021
…les that include the shims module from the all AI modules so that webpack can evaluate correctly #1509

Investigate removing the globals __extends() and __assign() populated by applicationinsights-shims #1280
MSNev added a commit that referenced this issue Apr 3, 2021
…les that include the shims module from the all AI modules so that webpack can evaluate correctly #1509

Investigate removing the globals __extends() and __assign() populated by applicationinsights-shims #1280
MSNev added a commit that referenced this issue Apr 14, 2021
…les that include the shims module from the all AI modules so that webpack can evaluate correctly #1509

Investigate removing the globals __extends() and __assign() populated by applicationinsights-shims #1280
MSNev added a commit that referenced this issue Apr 15, 2021
…les that include the shims module from the all AI modules so that webpack can evaluate correctly #1509

Investigate removing the globals __extends() and __assign() populated by applicationinsights-shims #1280
@MSNev MSNev added this to the 2.x.x (Next Release) milestone Apr 15, 2021
MSNev added a commit that referenced this issue Apr 15, 2021
…les that include the shims module from the all AI modules so that webpack can evaluate correctly #1509 (#1523)

Investigate removing the globals __extends() and __assign() populated by applicationinsights-shims #1280
@MSNev MSNev added the fixed - waiting release PR Committed and waiting deployment label Apr 15, 2021
@MSNev MSNev added released - NPM waiting - CDN deployment and removed fixed - waiting release PR Committed and waiting deployment labels Apr 23, 2021
@mmarshad
Copy link

@MSNev : installed the latest package but still having the issue
"@microsoft/applicationinsights-react-js": "^3.1.0",
"@microsoft/applicationinsights-web": "^2.6.2",

@MSNev
Copy link
Collaborator Author

MSNev commented Apr 26, 2021

We have not yet published a react update that uses v2.0.0 of the shims (should be later today), if you check your dependencies you should find that v1.0.3 is currently still getting included.

@MSNev
Copy link
Collaborator Author

MSNev commented Apr 26, 2021

PR to update the versions for release is out #1552

@MSNev
Copy link
Collaborator Author

MSNev commented Apr 26, 2021

React v3.1.1 and ReactNative v2.3.1 are now published to NPM

@MSNev
Copy link
Collaborator Author

MSNev commented Apr 26, 2021

I've deprecated the react v3.1.1 and react-native v2.3.1 packages until I can publish an updated versions #1553

@MSNev
Copy link
Collaborator Author

MSNev commented Apr 27, 2021

v2.6.2 is now fully deployed

@MSNev MSNev closed this as completed Apr 27, 2021
@MSNev
Copy link
Collaborator Author

MSNev commented Apr 28, 2021

I'm updating #1553 on the React and ReactNative releases with the current state, please see comments there for additional info.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants