DEV Community

naugtur
naugtur

Posted on • Edited on

Sometimes the best thing to do is an explicit nothing

This is a long story about why I wrote this line:

window.event = window.event
Enter fullscreen mode Exit fullscreen mode

react-dom under LavaMoat

While working on getting a project to work under @lavamoat/webpack I stumbled upon an issue where the whole thing would hang in Firefox (on my colleague's mac it would hang the entire browser)
After a bunch of inspector sorcery I found out that the app is throwing TypeError: can't redefine non-configurable property "event" faster than the browser can put together stacktraces for the errors.

The culprit for that was a line in react-dom

It's worth noting that contrary to what the comment says, Firefox implemented window.event in 2019 so me finding this takes me a step closer to an Archeology major.
Did I mention window.event is deprecated?

But why are we getting an error in Firefox and not in Chrome despite them both implementing window.event ?

The emulation LavaMoat does

This section assumes you already know basics of what LavaMoat does from lavamoat.github.io

When providing a new global for each package with just the specified fields, we need to make sure that they actually work. And in the browser it's fairly common to have fields on window that demand being on window because there's hidden "internal slots" on the objects the functions check.

What?

Let me explain with a code snippet

const a = { fetch }
a.fetch('/')
// TypeError: Failed to execute 'fetch' on 'Window': Illegal invocation
Enter fullscreen mode Exit fullscreen mode

Some functions demand that their this is the window. But the whole point of LavaMoat is to not give the real window with all globals to every package in dependencies.

So we need to give packages functions that wrap around functions and get/set pairs to then internally unwrap to run the actual native implementation with the right this.

So far so good.

But then there's scuttling.

In case a package gets enough references passed to it that it can reach the real window, after LavaMoat makes copies of all globals it's destroying everything on the actual globalThis by replacing it with a non-configurable getter that throws an error.

Cool, huh?

So that leaves us with react-dom running under LavaMoat with a policy that allows it write to window.event while the real window.event is long gone. Why the error tho?

window.event behavior

window.event is implemented with a setter that replaces itself with the value first time it's called. Why? Don't ask me, I didn't come up with it.

Let me repeat that in JavaScript in case you know it better than English ;)

Object.defineProperty(window, 'event', {
    set(value) {
      Object.defineProperty(this, 'event', { value })
    },
    configurable: true,
  })
Enter fullscreen mode Exit fullscreen mode

When we replace the original setter with the scuttling one (that's set to be configurable: false) the original setter (that we still have a copy of) will fail to overwrite the value.

Again, let me repeat that in JavaScript:

const es = Object.getOwnPropertyDescriptor(window, 'event').set
Object.defineProperty(window, 'event', { value:{a:1}, configurable: false })
Reflect.apply(es, window, [{a:2}])
Enter fullscreen mode Exit fullscreen mode

This fails silently in Chrome and loudly in FF

Conclusion

If spec was supposed to cover all details of the standard implementation it would have to go into detail infinitely.

Wait, what?

How is that a conclusion?

Ok, last step you need before you'll understand.

These are the setters in Chrome and Firefox respectively

Can you spot the difference between the two setters?

🎉 The one with arguments and caller is a sloppy mode function, so when it fails a defineProperty, it doesn't throw. 🎉

And this tiny detail in an implementation of an old deprecated property that react-dom keeps alive in its codebase in case someone uses a 20+ years old event handler in react is enough to make an entire project explode in one browser and work in another.

Takes me back to the early 2000s

Conclusion, for real this time

Now if you look at this line again, you should know what happens.

window.event = window.event
Enter fullscreen mode Exit fullscreen mode
  • a getter is called on event
  • its return value is passed to the setter
  • the setter replaces window.event with a value property
  • LavaMoat sees a value and avoids wrapping/unwrapping it at all
  • when code in a package does window.event = {} it won't leak to the real global

Update

I defaulted to assign the current value to the window.event to minimize the fragility if it becomes something else in the future. After some conversations under the links to this I posted to socials I think I'll go with window.event = undefined for LavaMoat, because there's a risk that it might capture an actual event object with references I don't want available to some of the packages.

Top comments (0)