search

sinclairzx81 / typebox

Json Schema Type Builder with Static Type Resolution for TypeScript
Other
5.08k stars 161 forks source link

Implement workaround for global built-in variable shadowing #1055

Closed itsyoboieltr closed 2 weeks ago

itsyoboieltr commented 3 weeks ago

Currently, TypeBox is not compatible with Babel (and thus all other frameworks that depend on it, like React Native, Expo, etc.), as Babel does not support the shadowing of global built-in variables.

More information about the issue can be found here:

TypeBox unfortunately does this intentionally in some parts of the codebase. Things like:

export const Object = _Object;

are illegal in Babel.

If we would like to support Babel, the following workarounds are suggested:

export var Object = _Object;
export { _Object as Object };
sinclairzx81 commented 3 weeks ago

@itsyoboieltr Hi, Thanks for this (and for chasing things up with the Babel team), that's really appreciated.

I will likely end up supporting one of these two options (likely the var declaration), however I am curious to see what they come back with respect to the globalThis qualifier mentioned on https://github.com/babel/babel/issues/16943#issuecomment-2453553812 first.

I'm guessing there is a reason they don't qualify the call (perhaps issues resolving the global environment on older ES versions). I think globalThis appeared on ES2020, but I assume if Babel can compile for ES2020 spec it could add the qualifier for the target (meaning users could just update to that spec for the issue to disappear). There may be other things to consider (I'm not a Babel user so I don't know all the details there unfortunately), but curious to see what they say.

Will prepare a PR for this update tomorrow.

Many thanks! S

sinclairzx81 commented 3 weeks ago

@itsyoboieltr Hi,

As suspected: https://github.com/babel/babel/issues/16943#issuecomment-2454032226

@itsyoboieltr Since globalThis doesn't exist in all environments, one possible check might be "undefined"!=typeof globalThis?globalThis:"undefined"!=typeof window?window:"undefined"!=typeof global?global:self, which does more harm than good.

It's probably not worth pursuing the issue with them any further (and assume configuring for the correct ES target to avoid excessive resolution of the environment is not supported by Babel). I think we can go ahead with the fix here.

Would you like to submit a PR for this? I think going with the following makes sense.

export var Object = _Object;

Let me know if you would like to, otherwise will take a look over the next few days. Also, thanks again for following up on this stuff across projects, it's been very much appreciated :)

Cheers! S

itsyoboieltr commented 2 weeks ago

@sinclairzx81 I tested the fix locally and it worked! I submitted the PR. Thank you!

sinclairzx81 commented 2 weeks ago

@itsyoboieltr Hiya,

Will close this one out as these updates have been merged and published on 0.33.18.

All the best! S