remove node types and bun types from workflows#199
Open
ernest-nowacki wants to merge 19 commits intomainfrom
Open
remove node types and bun types from workflows#199ernest-nowacki wants to merge 19 commits intomainfrom
ernest-nowacki wants to merge 19 commits intomainfrom
Conversation
…st that this setup actually works
product-security-plaid-production
bot
requested review from
mchain0 and
tomasz-ratajczyk-sc
… into feat/no-node-types
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is often coming back and throwing users off guard. Changes proposed in this PR aims to be solid first step to improve that experience.
Compile-time: Type-level restrictions
We're adding custom types to a whole set of common Node built-in modules (
node:crypto,node:fs,node:http,node:net, etc.). This custom type mark those APIs asneverwhich will result in clear errors when running workflows.Our
cre-sdk-exampleswill now present have nowWe also Removed
@types/nodeandbun-typesand ourcre-sdk-exampleswill now behave like user workflows now, correctly showing what is available.Build-time: Static analysis validator
There's an extra step added during workflow compilation that would check for usage of restricted node modules. It would produce readable errors instead of weird WASM traces.
What is not covered
Current implementation doesn't cover 3rd party libraries. For example if I import
my-custom-hasheslibrary which under the hoods is using unavailable NodeJS APIs we wouldn't be able to catch it as nicely as current direct node imports.That's because it would require
skipLibCheckoption set tofalsein TypeScript which is not a common practice and we could get more reports about unrelated 3rd party libs erroring out. I will keep exploring this - but I think current proposal already uplifts DX experience of the SDK a lot.