Skip to content

Commit

Permalink
Test for composite: true errors in CI, ensuring we can't ship those…
Browse files Browse the repository at this point in the history
… issues (#501)

## Summary
<!-- Succinctly describe your change, providing context, what you've
changed, and why. -->

We've seen some issues crop up when `composite: true` is present in
`tsconfig.json` files. This PR adds a `composite: true` project where we
test the Inngest package for compliance for the types that it exports.

We purposefully want to limit the number of types exported from the main
`"inngest"` entrypoint, as each type exported here becomes part of the
public API, where changing those types is a breaking change. Therefore,
the composite check gives us a (albeit inaccurate) test that can stop us
shipping these dangerous changes.

## Checklist
<!-- Tick these items off as you progress. -->
<!-- If an item isn't applicable, ideally please strikeout the item by
wrapping it in "~~"" and suffix it with "N/A My reason for skipping
this." -->
<!-- e.g. "- [ ] ~~Added tests~~ N/A Only touches docs" -->

- [ ] ~Added a [docs PR](https://github.com/inngest/website) that
references this PR~ N/A Bug fix and testing
- [x] Added unit/integration tests
- [x] Added changesets if applicable

## Related
<!-- A space for any related links, issues, or PRs. -->
<!-- Linear issues are autolinked. -->
<!-- e.g. - INN-123 -->
<!-- GitHub issues/PRs can be linked using shorthand. -->
<!-- e.g. "- inngest/inngest#123" -->
<!-- Feel free to remove this section if there are no applicable related
links.-->
- #384
- #385 
- #437
- #460
  • Loading branch information
jpwilliams committed Feb 26, 2024
1 parent 1b9f101 commit 0048c94
Show file tree
Hide file tree
Showing 12 changed files with 218 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .changeset/three-worms-hammer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"inngest": patch
---

Fix failures for `composite: true` errors
11 changes: 11 additions & 0 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,17 @@ jobs:
- uses: ./.github/actions/setup-and-build
- run: pnpm run test:deps

inngest_test_composite:
name: "inngest: Composite tests"
runs-on: ubuntu-latest
defaults:
run:
working-directory: packages/inngest
steps:
- uses: actions/checkout@v3
- uses: ./.github/actions/setup-and-build
- run: pnpm run test:composite

"eslint-plugin_test":
name: "eslint-plugin: Test"
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion packages/inngest/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module.exports = {
},
plugins: ["@typescript-eslint", "@inngest/internal", "import"],
root: true,
ignorePatterns: ["dist/", "*.d.ts", "*.js"],
ignorePatterns: ["dist/", "*.d.ts", "*.js", "test/"],
rules: {
"prettier/prettier": "warn",
"@inngest/internal/process-warn": "off",
Expand Down
28 changes: 26 additions & 2 deletions packages/inngest/etc/inngest.api.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions packages/inngest/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"test:types": "tsc --noEmit --project tsconfig.types.json --listFiles",
"test:dist": "tsc --noEmit dist/**/*.d.ts",
"test:deps": "tsx scripts/checkDependencies.ts",
"test:composite": "pnpm run local:pack && (cd test/composite_project && npm i ../../inngest.tgz && npm run test)",
"clean": "rm -rf ./dist",
"lint": "eslint .",
"postversion": "pnpm run build && pnpm run build:copy",
Expand Down
3 changes: 3 additions & 0 deletions packages/inngest/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export type {
export { ProxyLogger } from "./middleware/logger";
export type { LogArg } from "./middleware/logger";
export type {
BaseContext,
ClientOptions,
Context,
EventNameFromTrigger,
Expand All @@ -53,7 +54,9 @@ export type {
FunctionTrigger,
Handler,
LogLevel,
OutgoingOp,
RegisterOptions,
SendEventBaseOutput,
StepOptions,
StepOptionsOrId,
TimeStr,
Expand Down
14 changes: 14 additions & 0 deletions packages/inngest/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,12 @@ export const incomingOpSchema = z.object({
});

export type IncomingOp = z.output<typeof incomingOpSchema>;

/**
* The shape of a step operation that is sent to an Inngest Server from an SDK.
*
* @public
*/
export type OutgoingOp = Pick<
HashedOp,
"id" | "op" | "name" | "opts" | "data" | "error" | "displayName"
Expand Down Expand Up @@ -251,6 +257,12 @@ export type WithInvocation<T extends EventPayload> = Simplify<
{ name: T["name"] | `${internalEvents.FunctionInvoked}` } & Omit<T, "name">
>;

/**
* Base context object, omitting any extras that may be added by middleware or
* function configuration.
*
* @public
*/
export type BaseContext<
TOpts extends ClientOptions,
TTrigger extends keyof EventsFromOpts<TOpts> & string,
Expand Down Expand Up @@ -430,6 +442,8 @@ export type SendEventResponse = z.output<typeof sendEventResponseSchema>;

/**
* The response in code from sending an event to Inngest.
*
* @public
*/
export type SendEventBaseOutput = {
ids: SendEventResponse["ids"];
Expand Down
4 changes: 4 additions & 0 deletions packages/inngest/test/composite_project/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
node_modules
dist
tsconfig.tsbuildinfo
package-lock.json
36 changes: 36 additions & 0 deletions packages/inngest/test/composite_project/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Composite test project

## What is this?

This is a small project that uses `composite: true` in a `tsconfig.json` file.

Projects with this setting require that imported packages directly export all
types that are needed for inference from the entrypoint.

Every type we export is also part of the public API of a project; breaking
changes to those types are breaking changes for the package, too. Having a large
number of internal types exported at the entrypoint can make it very difficult
to avoid bumping the major version number.

## How do I use it?

You should test against this package by running `pnpm run test:composite` in
`packages/inngest`. This will:
- Build and pack `inngest`
- Install the packaged `inngest` into the composite project
- Attempt to build the project using `tsc`

Any inference errors will appear as a TS2742 error, like so:
```
src/index.ts:3:14 - error TS2742: The inferred type of 'inngest' cannot be named without a reference to '../node_modules/inngest/types'. This is likely not portable. A type annotation is necessary.
```

## Do I need to update it?

The main tests this project is performing is that types are correctly inferred
by the various functions exported by the `"inngest"`. Therefore, if adding new
API methods and calls, it's ideal to add them into this project.

This code is only built and is never run, so don't worry about it actually
working.

18 changes: 18 additions & 0 deletions packages/inngest/test/composite_project/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"name": "t_composite_test",
"version": "1.0.0",
"description": "",
"main": "index.js",
"scripts": {
"test": "tsc --build --force"
},
"keywords": [],
"author": "",
"license": "ISC",
"devDependencies": {
"typescript": "^5.3.3"
},
"dependencies": {
"inngest": "file:../../inngest.tgz"
}
}
62 changes: 62 additions & 0 deletions packages/inngest/test/composite_project/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import * as Inngest from "inngest";

export const inngest = new Inngest.Inngest({
id: "me",
schemas: new Inngest.EventSchemas().fromRecord<{
foo: { data: { foo: string } };
bar: { data: { bar: string } };
}>(),
middleware: [
new Inngest.InngestMiddleware({
name: "foo",
init() {
return {
onFunctionRun(ctx) {
console.log(ctx);

return {
transformInput(ctx) {
console.log(ctx);
},
afterExecution() {
console.log("afterExecution");
},
afterMemoization() {
console.log("afterMemoization");
},
beforeExecution() {
console.log("beforeExecution");
},
beforeMemoization() {
console.log("beforeMemoization");
},
beforeResponse() {
console.log("beforeResponse");
},
transformOutput(ctx) {
console.log(ctx);
},
};
},
onSendEvent() {
return {
transformInput(ctx) {
console.log(ctx);
},
transformOutput(ctx) {
console.log(ctx);
},
};
},
};
},
}),
],
});

void inngest.send({ name: "foo", data: { foo: "bar" } });

inngest.createFunction({ id: "my-fn" }, { event: "foo" }, async (ctx) => {
console.log(ctx);
return { foo: "bar" };
});
37 changes: 37 additions & 0 deletions packages/inngest/test/composite_project/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{
"compilerOptions": {
"baseUrl": ".",
"target": "es5",
"forceConsistentCasingInFileNames": true,
"composite": true,
"lib": ["dom", "dom.iterable", "esnext"],
"allowJs": true,
"skipLibCheck": false,
"strict": true,
"noEmit": false,
"outDir": "dist",
"esModuleInterop": true,
"module": "esnext",
"moduleResolution": "bundler",
"resolveJsonModule": true,
"isolatedModules": true,
"jsx": "preserve",
"incremental": true,
"typeRoots": ["./types"],
"plugins": [
{
"name": "next",
},
],
"paths": {
"~/*": ["./src/*"],
},
},
"include": [
"next-env.d.ts",
"src/**/*.ts",
"**/*.tsx",
".next/types/**/*.ts",
],
"exclude": ["node_modules"],
}

0 comments on commit 0048c94

Please sign in to comment.