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.

Sign up for GitHub

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

Jump to bottom

Use SWC with ts-node #3696

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Use SWC with ts-node #3696

wants to merge 1 commit into from

Conversation

mcmire
Copy link
Contributor

@mcmire mcmire commented Dec 22, 2023

Explanation

Currently, we use ts-node to run scripts written in TypeScript. By default ts-node has some overhead because it typechecks the code it runs. For instance, running the update-readme-content script takes around 2 seconds to run in full.

In other projects, we've solved this by integrating ts-node with SWC, which is faster because it avoids typechecking, a step we don't need to perform for scripts (as we can rely on yarn build in CI or TypeScript editor integration). This gets us to around 1.4 seconds for update-readme-content. This is still a lot for a simple script, but it is an improvement.

Changelog

(None)

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@mcmire
Use SWC with ts-node
9f28a25 
Currently, we use `ts-node` to run scripts written in TypeScript. By
default `ts-node` has some overhead because it typechecks the code it
runs. For instance, running the `update-readme-content` script takes
around 2 seconds to run in full.

In other projects, we've solved this by integrating `ts-node` with SWC,
which is faster because it avoids typechecking, a step we don't need to
perform for scripts (as we can rely on `yarn build` in CI or TypeScript
editor integration). This gets us to around 1.4 seconds for
`update-readme-content`. This is still a lot for a simple script, but it
is an improvement.
@mcmire mcmire requested a review from a team as a code owner December 22, 2023 17:34
@mcmire mcmire force-pushed the use-swc-with-ts-node branch from b1dae22 to 9f28a25 Compare December 22, 2023 17:34
@socket-security Socket Security
Copy link

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
@swc/core 1.3.101 filesystem, shell, environment +12 407 MB kdy1

@socket-security Socket Security
Copy link

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Issue Package Version Note Source
Shell access @swc/core 1.3.101
Unpublished package @swc/types 0.1.5
  • Version: 1/5/2000, 12:00:00 AM

Next steps

What is shell access?

This module accesses the system shell. Accessing the system shell increases the risk of executing arbitrary code.

Packages should avoid accessing the shell which can reduce portability, and make it easier for malicious shell access to be introduced.

What are unpublished packages?

Package version was not found on the registry. It may exist on a different registry and need to be configured to pull from that registry.

Packages can be removed from the registry by manually un-publishing, a security issue removal, or may simply never have been published to the registry. Reliance on these packages will cause problem when they are not found.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore [email protected] bar@* or ignore all packages with @SocketSecurity ignore-all

@Gudahtt
Copy link
Member

Gudahtt commented Dec 22, 2023

In other projects, we've solved this by integrating ts-node with SWC, which is faster because it avoids typechecking, a step we don't need to perform for scripts (as we can rely on yarn build in CI or TypeScript editor integration)

yarn build won't build scripts though, right? It only builds packages. It seems that that would leave these scripts without type checking in CI.

Using SWC makes sense but we should ensure we still get these type checked somewhere. e.g. maybe as part of the lint step.

@mcmire
Copy link
Contributor Author

mcmire commented Dec 22, 2023 •
edited
Loading

Yeah, maybe this isn't a good idea right now. The way that we've structured our TypeScript config is pretty confusing at the moment and I'm actually not sure how any of the scripts are getting typechecked at all (considering that they aren't included in the root tsconfig.json). I think we'd want to make that easier to reason about so we can ensure it's doing the right thing before merging this.

@mcmire mcmire marked this pull request as draft December 22, 2023 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mcmire @Gudahtt