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

define globalThis if not defined #89

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

artursapek
Copy link

I'm using this plugin in a project that started throwing errors in some sub-dependency because globalThis wasn't defined yet when this code ran. This one-line change resolves those errors. :-)

@artursapek
Copy link
Author

yo @davidmyersdev any feedback?

@davidmyersdev
Copy link
Owner

Hey @artursapek. 👋 I just ran the tests, and it looks like some are failing and need to be updated. Would you mind updating those? I can merge once CI is passing.

@baoskee
Copy link

baoskee commented May 22, 2024

Screenshot 2024-05-22 at 4 57 15 PM

This only happens in HMR mode on vite dev. In addition to globalThis error @artursapek mentioned, I also get this error after setting globalThis to {}.

@unixhelloworld
Copy link

unixhelloworld commented Jul 3, 2024

@artursapek you need to update the test files to pass the tests;

Test files:


Add this line for all tests

globalThis = globalThis || window

Your branch

expect(result?.code).toEqual(formatWhitespace(`
  import __buffer_polyfill from "/shims/buffer/dist/index.js"
  import __global_polyfill from "/shims/global/dist/index.js"
  import __process_polyfill from "/shims/process/dist/index.js"
  globalThis = globalThis || window
  globalThis.Buffer = globalThis.Buffer || __buffer_polyfill
  globalThis.global = globalThis.global || __global_polyfill
  globalThis.process = globalThis.process || __process_polyfill

  Buffer.from("test");
`))

@artursapek also, please update your branch, as there are already many changes in the tests in the main branch that may affect the final result.


Main branch

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.

None yet

4 participants