Skip to content

Conversation

jumski
Copy link
Contributor

@jumski jumski commented Oct 6, 2025

TL;DR

Improved TypeScript type checking with stricter validation and fixed map return type inference in the Flow DSL.

What changed?

  • Added a new typecheck-strict.sh script that performs two-pass type checking:
    1. Project-wide type check
    2. Individual file checks to catch unused @ts-expect-error directives
  • Updated project.json files to use the new script for type checking
  • Made test:types a dependency of the main test target in multiple packages
  • Fixed type inference issues in the Flow DSL's map method by using AwaitedReturn<THandler>[] instead of any[]
  • Added new type tests in map-return-type-inference.test-d.ts to verify proper type inference
  • Cleaned up and improved existing type tests

How to test?

  1. Run the type checking with the new script:

    ./scripts/typecheck-strict.sh
  2. Run the type tests to verify the fixed map return type inference:

    pnpm nx test:types dsl
  3. Verify that the main test command now includes type checking:

    pnpm nx test dsl
    pnpm nx test client
    pnpm nx test core

Why make this change?

The Flow DSL had a type inference bug where map step return types were incorrectly inferred as any[] instead of preserving the specific return type structure. This made it difficult to catch type errors in subsequent steps that used the map output. The new strict type checking process also helps catch unused @ts-expect-error directives, which can hide real type errors when they're no longer needed.

Copy link

changeset-bot bot commented Oct 6, 2025

⚠️ No Changeset found

Latest commit: 09fe21a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor Author

jumski commented Oct 6, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • merge:queue - adds this PR to the back of the merge queue
  • hotfix:queue - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link

nx-cloud bot commented Oct 6, 2025

View your CI Pipeline Execution ↗ for commit 09fe21a

Command Status Duration Result
nx run-many -t build --projects client,dsl --co... ✅ Succeeded 7s View ↗
nx affected -t build --configuration=production... ✅ Succeeded 4s View ↗
nx affected -t lint typecheck test --parallel -... ✅ Succeeded 7m 5s View ↗

☁️ Nx Cloud last updated this comment at 2025-10-07 08:09:24 UTC

@jumski jumski force-pushed the 10-06-fix-typecheck-tests branch from f18b09a to 569aad4 Compare October 7, 2025 06:34
Copy link
Contributor

github-actions bot commented Oct 7, 2025

🔍 Preview Deployment: Website

Deployment successful!

🔗 Preview URL: https://pr-231.pgflow.pages.dev

📝 Details:

  • Branch: 10-06-fix-typecheck-tests
  • Commit: 13403774cb49f2ab4678cdff331ee7369bf7affd
  • View Logs

_Last updated: _

Copy link
Contributor

github-actions bot commented Oct 7, 2025

🔍 Preview Deployment: Playground

Deployment successful!

🔗 Preview URL: https://pr-231--pgflow-demo.netlify.app

📝 Details:

  • Branch: 10-06-fix-typecheck-tests
  • Commit: 13403774cb49f2ab4678cdff331ee7369bf7affd
  • View Logs

_Last updated: _

Copy link
Contributor

graphite-app bot commented Oct 7, 2025

Merge activity

  • Oct 7, 8:28 AM UTC: jumski added this pull request to the Graphite merge queue.
  • Oct 7, 8:29 AM UTC: CI is running for this pull request on a draft pull request (#232) due to your merge queue CI optimization settings.
  • Oct 7, 8:29 AM UTC: Merged by the Graphite merge queue via draft PR: #232.

graphite-app bot pushed a commit that referenced this pull request Oct 7, 2025
### TL;DR

Improved TypeScript type checking with stricter validation and fixed map return type inference in the Flow DSL.

### What changed?

- Added a new `typecheck-strict.sh` script that performs two-pass type checking:
  1. Project-wide type check
  2. Individual file checks to catch unused `@ts-expect-error` directives
- Updated project.json files to use the new script for type checking
- Made `test:types` a dependency of the main `test` target in multiple packages
- Fixed type inference issues in the Flow DSL's map method by using `AwaitedReturn<THandler>[]` instead of `any[]`
- Added new type tests in `map-return-type-inference.test-d.ts` to verify proper type inference
- Cleaned up and improved existing type tests

### How to test?

1. Run the type checking with the new script:
   ```bash
   ./scripts/typecheck-strict.sh
   ```

2. Run the type tests to verify the fixed map return type inference:
   ```bash
   pnpm nx test:types dsl
   ```

3. Verify that the main test command now includes type checking:
   ```bash
   pnpm nx test dsl
   pnpm nx test client
   pnpm nx test core
   ```

### Why make this change?

The Flow DSL had a type inference bug where map step return types were incorrectly inferred as `any[]` instead of preserving the specific return type structure. This made it difficult to catch type errors in subsequent steps that used the map output. The new strict type checking process also helps catch unused `@ts-expect-error` directives, which can hide real type errors when they're no longer needed.
@graphite-app graphite-app bot closed this Oct 7, 2025
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.

1 participant