Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[BUG] "typeof window" can not be the define key. #1954

Closed
Amour1688 opened this issue Jan 24, 2022 · 3 comments
Closed

[BUG] "typeof window" can not be the define key. #1954

Amour1688 opened this issue Jan 24, 2022 · 3 comments

Comments

@Amour1688
Copy link

  • esbuild

image

  • webpack

image

@upupming
Copy link

upupming commented Jan 24, 2022

@Amour1688 I think currently we can only use --define:window=undefined directly instead of --define:"typeof window"=undefined:

Input:

if (typeof window === 'undefined') {
  console.log('sss')
} else {
  console.log('ttt')
}
console.log(window)

esbuild test.js --define:window=undefined Output:

if (true) {
  console.log("sss");
} else {
  console.log("ttt");
}
console.log(void 0);

esbuild test.js --define:window=\"object\" Output:

if (false) {
  console.log("sss");
} else {
  console.log("ttt");
}
console.log("object");

However this will change the value of window in the compiled output, actually we only want to define typeof window, so that we can run something only on browser-side or node-side, just like process.browser and typeof window !== 'undefined' in Next.js: vercel/next.js#2473 (comment), https://github.com/vercel/next.js/blob/0fb1183973f6b818ce2972a64570ee566cbd7357/packages/next/build/babel/loader/get-config.ts#L105

@evanw
Copy link
Owner

evanw commented Jan 24, 2022

This is not a bug. The define feature is only for replacing certain kinds of expressions. It's not a general-purpose code replacement mechanism. Changing just typeof window without changing window sounds potentially unsafe since it means code will have an inconsistent view of what the window variable is. You are welcome to define process.browser or some other thing instead.

@evanw
Copy link
Owner

evanw commented Jan 27, 2022

Closing this issue because this is not a bug, and I don't think this feature request should be implemented because it leads to logical inconsistencies in the code that could introduce bugs and/or be confusing.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants