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

fix: workaround undefined global this #688

Closed

Conversation

yujiosaka
Copy link

Note: the engine.io.js file is the generated output of make engine.io.js, and should not be manually modified.

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other

Current behaviour

Default export of globalThis seems to have a problem
in the "browser" field when the library is loaded asynchronously.

We are using engine.io-client via socket.io-client.
We recently upgraded sockete.io-client from v3.1.3 to v4.4.1.

And we started seeing the following error:
Screen Shot 2022-04-22 at 11 25 05

globalThis is undefined here:

const terminationEvent = "onpagehide" in globalThis ? "pagehide" : "unload";

We've confirmed that the issue started from socket.io-client v4.3.0,
which is the version where both socket.io-client and engine.io-client
migrated their builds from webpack to rollup.

So we are suspecting it's the compatibility issue of mixing webpack and rollup.

New behaviour

Avoid using default import resolves the issue.

Other information (e.g. related issues)

this might be related:
4971914

@yujiosaka
Copy link
Author

will fix the test later

Default export of globalThis seems to have a problem
in the "browser" field when the library is loaded asynchronously
with webpack, so this is a quick workaround.
@yujiosaka yujiosaka force-pushed the workaround-undefined-global-this branch from 8dec9e1 to 6ee02ac Compare April 22, 2022 04:25
@darrachequesne
Copy link
Member

Hi! I wasn't able to reproduce the issue: https://github.com/socketio/socket.io/tree/main/examples/webpack-build

Which version of webpack are you using? Could you please provide a complete example?

@yujiosaka
Copy link
Author

@darrachequesne
let me try, but i'm also having trouble providing a minimum reproducible example.
we're not managing 100% of our code, and our code is embedded in our customers' environment.
so we haven't figured out yet which configuration, which version, which combination is causing the issue.

we are using webpack 3.12.0

@yujiosaka
Copy link
Author

@darrachequesne
it turned out it was caused by a global window object pollution.

i found that __esModule: true was unintentionally added to the global window object.

Screen Shot 2022-04-25 at 8 18 21

it's not caused by socket.io-client/engine.io-client. it's caused by our customers' environments.

when importing an object in esm, babel tries to find if the imported module has the __esModule property.
and the behavior is different depending on whether there is __esModule is defined in the imported object.

_interopRequireDefault is a function added by babel.

Screen Shot 2022-04-25 at 8 21 57

you can see that when __esModule is truthy, it tries to import the entire object
rather than wrapping the object by { default: obj }.

thus, the following code throws an error because _globalThis2.default becomes undefined.

Screen Shot 2022-04-25 at 8 38 37

i also spent some time figuring out what was causing the global pollution and how common the problem is.
and i found that typescript used to have this issue. so i'm suspecting our customer is using an old version of it.

apollographql/graphql-tag#412
microsoft/tslib#32

currently 0.6% of our customers is having this issue when importing socket.io-client/engine.io-client.
after running whole the investigation above, i understand if this pr is not accepted for such a rare case.

but i do appreciate if you merge this change because for our business
we are just providing our javascript tag to our customers (like google analytics tag)
and we have no control over which library customers are using

darrachequesne pushed a commit that referenced this pull request May 2, 2022
Default export of globalThis seems to have a problem in the "browser"
field when the library is loaded asynchronously with webpack.
@darrachequesne
Copy link
Member

Merged as 32878ea, and included in [email protected]. Thanks a lot for the work on this ❤️

@darrachequesne darrachequesne added this to the 6.6.2 milestone May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants