Post-build validation of typescript files

[mrginglymus]

Is your feature request related to a problem? Please describe.

In #31527, I observed tsup generating malformed typescript definitions.

This is a known risk of using tsup - they recommend testing the output with either tsc or @arethetypeswrong/cli after generation.

Describe the solution you'd like

A post-bundle validation step that type-checks generated .d.ts files.

In theory this could be done with a tsconfig file that looks something like:

{
  "extends": "../../tsconfig.json",
  "compilerOptions": {
    "skipLibCheck": false
  },
  "include": ["dist/**/*.d.ts"],
  "exclude": [ "../../**/node_modules"]
}

However! In order to test definition files, it requires skipLibCheck to be false. This will precipitate a whole bunch of unrelated errors in downstream packages that have invalid or incompatible types.

Describe alternatives you've considered

No response

Are you able to assist to bring the feature to reality?

yes, I can

Additional context

No response

[mrginglymus]

Possibly fixed by #31555

[Sidnioulz]

We've discussed this in the weekly triage meeting with @ndelangen and @mrginglymus, and came to the conclusion that it makes a lot of sense to run type checks against sandboxes in the CI, as a new job.

@mrginglymus would like to give implementation a try.

[Sidnioulz]

@mrginglymus I think you could also create your own type-check-sandboxes (or check-sandboxes) if you don't want your tests tied to other tests that might have a good reason to be disabled again in the future. We do have check jobs already that do type checking on the monorepo.

[ndelangen]

I worry a bit that we'd run into type issues reported that aren't caused by storybook's types being incorrect, but rather other libraries.

Now I might still be good to be aware of those, maybe?

I'd say don't touch smoke-test-sandboxes and instead add a new type-check-sandboxes job?

[Sidnioulz]

IIRC Bill wanted to only include NPM dependencies starting with storybook/ or @storybook/. If there are type errors from 3rd-party code in these, it'd have to mean that our upstream made breaking changes we haven't yet accounted for, wouldn't it?

[mrginglymus]

Yes - something like this

storybook/scripts/check/utils/typescript.ts

Line 11 in 1abc1a5

typescript.getPreEmitDiagnostics(program).filter((d) => d.file?.fileName.startsWith(cwd)),

which presumably is there to ensure only issues from the current module are recorded, but could easily be modified to suit this need.

Though I think at least the React ecosystem is pretty solid in terms of not having issues.

I'll try adding a new type-check-sandboxes job.

[shilman]

No recollection @Sidnioulz. Maybe because they are redundant with some other tests and CI is really expensive?