Skip to content

Conversation

@rekmarks
Copy link
Member

@rekmarks rekmarks commented Sep 17, 2024

Adds build-time type checks to Vite via vite-plugin-checker, which we configure to run tsc as part of the Vite pipeline.

As part of this, the lint:types script introduced in #73 (f.k.a. build:types, which was a misnomer) is removed. In essence, I first attempted to establish a lint:types script common across all packages. Any monorepo-wide implementation thereof turns out to be equivalent to just running a full, declaration-only TypeScript build of the entire monorepo (see the TypeScript project references documentation for details). This being the case, rather than using a one-off script for @ocap/extension, I found a Vite plugin that allows us to check types as part of the build process, as we do everywhere else.

@socket-security
Copy link

socket-security bot commented Sep 17, 2024

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

Package New capabilities Transitives Size Publisher
npm/vite-plugin-checker@0.8.0 environment, network Transitive: filesystem, shell +22 3.06 MB fi3ework

View full report↗︎

@socket-security
Copy link

socket-security bot commented Sep 17, 2024

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring: npm/vite-plugin-checker@0.8.0, npm/vscode-jsonrpc@6.0.0, npm/vscode-languageclient@7.0.0, npm/vscode-languageserver@7.0.0

View full report↗︎

Next steps

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 ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

@rekmarks
Copy link
Member Author

@SocketSecurity ignore npm/vscode-jsonrpc@6.0.0 ignore npm/vscode-languageclient@7.0.0 ignore npm/vscode-languageserver@7.0.0 ignore npm/vite-plugin-checker@0.8.0

vite-plugin-checker is the dev dependency added in this PR, and the rest are from its direct dependencies and are developed by Microsoft.

"build:clean": "yarn clean && yarn build",
"build:docs": "yarn workspaces foreach --all --exclude @ocap/monorepo --exclude @ocap/extension --parallel --interlaced --verbose run build:docs",
"build:source": "yarn workspaces foreach --all --topological --parallel --interlaced --exclude @ocap/monorepo --verbose run build",
"build:watch": "yarn run build --watch",
Copy link
Member Author

Choose a reason for hiding this comment

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

This script had been broken forever and is probably a lost cause.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's see if we end up introducing some pivot on this down the road 🤞

},
"references": [{ "path": "../test-utils" }],
"include": ["./src", "test/envelope-kit-fixtures.ts"]
"include": ["./src", "./test/envelope-kit-fixtures.ts"]
Copy link
Member Author

Choose a reason for hiding this comment

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

Just for consistency.

@rekmarks rekmarks enabled auto-merge (squash) September 17, 2024 23:16
Copy link
Contributor

@SMotaal SMotaal left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

"build:clean": "yarn clean && yarn build",
"build:docs": "yarn workspaces foreach --all --exclude @ocap/monorepo --exclude @ocap/extension --parallel --interlaced --verbose run build:docs",
"build:source": "yarn workspaces foreach --all --topological --parallel --interlaced --exclude @ocap/monorepo --verbose run build",
"build:watch": "yarn run build --watch",
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's see if we end up introducing some pivot on this down the road 🤞

jsTrustedPrelude({
trustedPreludes: jsTrustedPreludes,
}),
viteChecker({ typescript: { tsconfigPath: 'tsconfig.build.json' } }),
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed type-checking came last, and tried to reorder the plugins to bail early on failure without luck. It is not an issue of urgency for now, and if need be, we may need to revisit this down the road.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what the behavioral significance is of the order of the plugins array, but yeah, what I have seems to work.

@rekmarks rekmarks merged commit abeac03 into main Sep 17, 2024
@rekmarks rekmarks deleted the rekm/type-checking branch September 17, 2024 23:50
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