Skip to content

fix(build): let top-level this refer to globalThis#5312

Merged
patak-cat merged 2 commits intovitejs:mainfrom
aleclarson:fix/context-window
Oct 21, 2021
Merged

fix(build): let top-level this refer to globalThis#5312
patak-cat merged 2 commits intovitejs:mainfrom
aleclarson:fix/context-window

Conversation

@aleclarson
Copy link
Copy Markdown
Contributor

@aleclarson aleclarson commented Oct 15, 2021 •
edited
Loading

Description

This prevents THIS_IS_UNDEFINED warning from Rollup when @babel/plugin-transform-react-jsx emits jsx calls that pass this inside an arrow function component, which in turn leads to SOURCEMAP_ERROR warnings, because the this reference doesn't exist in the original code.

An example Rollup warning message for SOURCEMAP_ERROR:

Error when using sourcemap for reporting an error: Can't resolve original location of error.

How code output is changed

This PR is especially important for @vitejs/plugin-react.

  return /* @__PURE__ */ jsxDevRuntime.exports.jsxDEV(Provider, {
    value: props,
    children
  }, void 0, false, {
    fileName: _jsxFileName$z,
    lineNumber: 42,
    columnNumber: 26
  }, this); // <== The problem

…becomes…

  return /* @__PURE__ */ jsxDevRuntime.exports.jsxDEV(Provider, {
    value: props,
    children
  }, void 0, false, {
    fileName: _jsxFileName$z,
    lineNumber: 42,
    columnNumber: 26
  }, window); // <== The solution

Workaround

Ignore the warning, or set rollupOptions.onwarn to handle it manually.

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@aleclarson
fix: let top-level this refer to window
a7f4a61 
This prevents THIS_IS_UNDEFINED warning from Rollup when @babel/plugin-transform-react-jsx emits `jsx` calls that pass `this` inside an arrow function component, which in turn leads to SOURCEMAP_ERROR warnings, because the `this` reference doesn't exist in the original code.
@aleclarson aleclarson added the p2-edge-case Bug, but has workaround or limited in scope (priority) label Oct 15, 2021
@aleclarson
Copy link
Copy Markdown
Contributor Author

aleclarson commented Oct 15, 2021

Hmm, I wonder if we should use "globalThis" in SSR env? @patak-js

@patak-cat
Copy link
Copy Markdown
Member

patak-cat commented Oct 15, 2021

@aleclarson I think we should use globalThis for non-legacy. It is supported from Node 12. The legacy plugin should take of transpiling it down to window when necessary, no?

@jordanamr
Copy link
Copy Markdown

jordanamr commented Oct 18, 2021

Is there a way to silence this whilst waiting on this PR merge ? This is spamming my build logs with thousands of lines ahah

@aleclarson
Copy link
Copy Markdown
Contributor Author

aleclarson commented Oct 18, 2021

@jordanamr Set rollupOptions.context to "window" or "globalThis" in your Vite config

@antfu
Copy link
Copy Markdown
Member

antfu commented Oct 18, 2021

I would vote for globalThis

patak-cat
patak-cat requested changes Oct 20, 2021
View reviewed changes
@aleclarson aleclarson changed the title fix: let top-level this refer to window fix: let top-level this refer to globalThis Oct 20, 2021
@aleclarson aleclarson changed the title fix: let top-level this refer to globalThis fix(build): let top-level this refer to globalThis Oct 20, 2021
@patak-cat patak-cat merged commit 7e25429 into vitejs:main Oct 21, 2021
@genffy
Copy link
Copy Markdown

genffy commented Nov 10, 2021

this fix cause anthoer issue, will convert inputPath with global keyword to gloalThis nuxt/nuxt#11958

@patak-cat
Copy link
Copy Markdown
Member

patak-cat commented Nov 10, 2021

Could you open a new issue so this csn be properly tracked?

@genffy
Copy link
Copy Markdown

genffy commented Nov 10, 2021

Could you open a new issue so this csn be properly tracked?

sorry, comment metioned issue root casue is nuxt/bridge#204

@patak-cat
Copy link
Copy Markdown
Member

patak-cat commented Nov 10, 2021

If you think there is something to fix in Vite, please open an issue here

@aleclarson
Copy link
Copy Markdown
Contributor Author

aleclarson commented Nov 10, 2021

@genffy That issue is unrelated to this PR.

@vitejs vitejs locked and limited conversation to collaborators Nov 10, 2021
@aleclarson aleclarson deleted the fix/context-window branch February 25, 2022 20:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@patak-cat patak-cat Awaiting requested review from patak-cat

Assignees

No one assigned

Labels

p2-edge-case Bug, but has workaround or limited in scope (priority)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@aleclarson @patak-cat @jordanamr @antfu @genffy