-
-
Notifications
You must be signed in to change notification settings - Fork 638
Description
Problem
PR #2125 changed TypeScript types in packages/react-on-rails/src/types/index.ts (changing ReactElement<unknown> to ReactElement), which exposed a latent type error in serverRenderReactComponent.ts. This wasn't caught until the release script ran pnpm publish which performs a fresh TypeScript build.
The generator/example tests would have caught this because they run a full build, but they weren't triggered because the CI change detection doesn't consider packages/react-on-rails/** changes as requiring generator tests.
Root Cause
In script/ci-changes-detector (or the workflow's change detection logic), changes to packages/react-on-rails/src/** files don't trigger the generator/examples tests. These tests do a full build and would catch TypeScript compilation errors.
Proposed Solution
Update the CI change detection to run generator tests (or at minimum, a TypeScript build verification) when non-doc files in packages/react-on-rails/ change.
Specifically, changes to these paths should trigger additional CI:
packages/react-on-rails/src/**/*.tspackages/react-on-rails/tsconfig.json- Root
tsconfig.json
Context
- Fixed in PR Fix TypeScript error in processPromise return type #2204
- The latent bug was introduced in beta.11 (
3758d1a1d) but only became visible when types changed in PR Move React/Shakapacker version compatibility to generator smoke tests #2125 - The
lint-js-and-ruby.ymlworkflow does runpnpm run type-check, but this passed at beta.11 becauseReactElement<unknown>was more permissive thanReactElement
Related
- PR Move React/Shakapacker version compatibility to generator smoke tests #2125 - Changed
ReactElement<unknown>toReactElement - PR Fix TypeScript error in processPromise return type #2204 - Fixed the type error