Incorrect polyfill __knownSymbol lookups for Symbol.dispose (and probably others)
thodnev opened this issue · 2 comments
Problem statement
Esbuild uses __knownSymbol
in generated code for Symbol
lookups, that relies on Symbol.for()
esbuild/internal/runtime/runtime.go
Line 78 in d34e79e
However, the property
dispose
does not get set on global Symbol
by the generated code.This results in
Symbol.dispose
resolving to undefined
in the generated code, thus making the objects non-disposable.Which further causes the
TypeError: Object not disposable
runtime error (Firefox 131.0b9 & Chromium 129.0.6668.58 tested, but sure the others will fail as well). Error is produced by this fragment:esbuild/internal/runtime/runtime.go
Lines 510 to 514 in d34e79e
Temporary (and hackish) solutions right now
One possible solution will be to polyfill the Symbol.dispose
in TS ourselves, like the following:
Symbol.dispose ??= Symbol.for('Symbol.dispose')
But:
- It does not correspond to the polyfill provided in the official TS documentation:
(the one provided there simply won't work, as per Symbol spec, the
Symbol.dispose ??= Symbol("Symbol.dispose");
Symbol(key)
is guaranteed to return a distinct object every time, even for the same key - It is very obscure to find a correct solution, as it requires digging inside toolchain internals
- Why does the user have to polyfill it himself everywhere, yet more, in his source files
Proper fix proposal
Add to the generated code a procedure to test&set dispose
property on Symbol
object, something like:
if (Symbol.dispose === undefined) {
Object.defineProperty(Symbol, 'dispose', {value: Symbol.for('Symbol.dispose')})
}
Examples
Generated code (as of now):
- Test TS fragment:
class TestCls implements Disposable { [Symbol.dispose]() { console.log('I was disposed') } }
- Output JS fragment built with
esbuild dispose.ts --target=es2022 --outfile=dispose.js
class TestCls { [Symbol.dispose]() { console.log("I was disposed"); } }
Please find the TS input and the corresponding Esbuild JS output attached
(built with esbuild dispose.ts --target=es2022 --outfile=dispose.out.js
) :
dispose.ts.txt
dispose.out.js.txt
Sorry, I don't understand. You need to polyfill Symbol.dispose
before you use it with esbuild, just like all other new APIs. This is because esbuild transforms newer syntax to older syntax for you but does not polyfill APIs for you. And polyfilling Symbol.dispose
should already work fine. You can do it however you like. For example, you can do it exactly as TypeScript's documentation tells you to do it (as you pointed out):
Symbol.dispose ??= Symbol("Symbol.dispose");
function demo() {
class Foo {
constructor() { console.log('constructor') }
[Symbol.dispose]() { console.log('dispose') }
}
using foo = new Foo
}
demo()
If you build this with esbuild and run it, it will output the following:
constructor
dispose
So everything already appears to be working correctly. This happens because __knownSymbol
just returns the value of Symbol[name]
if it's present. If you polyfill Symbol.dispose
before using it, then Symbol['dispose']
will be your polyfill. The stuff with Symbol.for
is irrelevant for your use case. It's only there for obscure compatibility reasons with code generated by Babel.
You have to polyfill Symbol.dispose
with esbuild just like you have to do with TypeScript. This is already in esbuild's documentation:
Note that this is only concerned with syntax features, not APIs. It does not automatically add polyfills for new APIs that are not used by these environments. You will have to explicitly import polyfills for the APIs you need (e.g. by importing core-js). Automatic polyfill injection is outside of esbuild's scope.
@evanw Thank you for the clarification.
In the case the user has not provided his own polyfill, maybe there should be a hint in the error message. This would largely reduce uncertainty for the user.
Now the error message states TypeError: Object not disposable
. Which leads to doubts that probably there is something wrong with the object itself.
What about TypeError: Object not disposable or Symbols.dispose not provided
?
UPD:
Tested it with the core-js/modules/esnext.symbol.dispose
polyfill, and it works as well.
But core-js introduced a lot of code into the bundle (+8.1 KiB pure, +3 KiB gzipped, +2.7 KiB brotli). So for the sole purpose of having this feature, I'll probably stick with a simpler TS-native polyfill
Symbol.dispose ??= Symbol.for('Symbol.dispose')