Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Typescript codegen for Shims #262

Merged
merged 8 commits into from
Oct 3, 2023
Merged

Typescript codegen for Shims #262

merged 8 commits into from
Oct 3, 2023

Conversation

namesty
Copy link
Contributor

@namesty namesty commented Oct 2, 2023

This PR aims to provide better shims updates and type/build checks using TS.

It organizes shims into a shims directory. Inside of said directory there's a globals directory. There's where all JS global shims live as separate TS files.

Pros

  • Type-checking for shims code. If it doesn't build with TS, it will throw.
  • Separation of concerns. Each shim lives in its own separate file as its own separate variable
  • Easier debugging. Shim behavior can be debugged more easily as you can now intervene the shims code in Typescript, instead of modifying the JS string we had before

Pain points

This is def better than the mega string manipulation we had before, but it's still far from ideal:

Because of how the bundler processes globals + how we inject code into the engine wrap + the need to shim require for the JS engine (instead of shimming just modules):

  • Required modules need to be local to the require global. See how axios, fs , etc. are done.
  • We require a name mapping of global -> shim variable name. See shims/globals/index.ts

@namesty namesty changed the title Added TS shims + codegen script Typescript codegen for Shims Oct 2, 2023
@namesty namesty marked this pull request as ready for review October 2, 2023 17:42
Copy link
Contributor

@dOrgJelli dOrgJelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@dOrgJelli dOrgJelli merged commit 6272435 into dev Oct 3, 2023
@cbrzn cbrzn deleted the namesty/better-shims branch December 6, 2023 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants