Skip to content

Conversation

@jakebailey
Copy link
Member

Seeing how bad the impact of not fixing microsoft/typescript-go#1080 is.

@jakebailey
Copy link
Member Author

@typescript-bot run dt
@typescript-bot user test this
@typescript-bot test top800

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 28, 2025

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
run dt ✅ Started 👀 Results
user test this ✅ Started ✅ Results
test top800 ✅ Started 👀 Results

@typescript-bot
Copy link
Collaborator

Hey @jakebailey, the results of running the DT tests are ready.

There were interesting changes:

Branch only errors:

Package: rdfjs__fetch
Error:

Error: 
/mnt/vss/_work/1/DefinitelyTyped/types/rdfjs__fetch/rdfjs__fetch-tests.ts
  42:5  error  TypeScript@local compile error: 
Type 'Stream<Quad>' is not assignable to type 'Stream<QuadExt>'.
  The types returned by 'read()' are incompatible between these types.
    Type 'Quad | null' is not assignable to type 'QuadExt | null'.
      Property 'toCanonical' is missing in type 'Quad' but required in type 'QuadExt'  @definitelytyped/expect

✖ 1 problem (1 error, 0 warnings)

    at combineErrorsAndWarnings (/mnt/vss/_work/1/DefinitelyTyped/node_modules/.pnpm/@definitelytyped+dtslint@0.2.34_typescript@6.0.0-dev.20251028/node_modules/@definitelytyped/dtslint/dist/index.js:199:28)
    at runTests (/mnt/vss/_work/1/DefinitelyTyped/node_modules/.pnpm/@definitelytyped+dtslint@0.2.34_typescript@6.0.0-dev.20251028/node_modules/@definitelytyped/dtslint/dist/index.js:191:20)

You can check the log here.

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the user tests with tsc comparing main and refs/pull/62684/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Git clone failed"

Otherwise...

Everything looks good!

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the top 800 repos with tsc comparing main and refs/pull/62684/merge:

Something interesting changed - please have a look.

Details

continuedev/continue

13 of 19 projects failed to build with the old tsc and were ignored

binary/tsconfig.json

  • error TS2345: Argument of type 'ZodObject<Pick<extendShape<{ eventName: ZodString; schema: ZodString; timestamp: ZodString; userId: ZodString; userAgent: ZodString; selectedProfileId: ZodString; }, { ...; }>, "timestamp" | ... 29 more ... | "useFileSuffix">, "strip", ZodTypeAny, { ...; }, { ...; }> | ... 11 more ... | ZodObject<...>' is not assignable to parameter of type 'AnyZodObject'.
  • error TS2345: Argument of type 'ZodObject<Pick<extendShape<{ eventName: ZodString; schema: ZodString; timestamp: ZodString; userId: ZodString; userAgent: ZodString; selectedProfileId: ZodString; }, { ...; }>, "timestamp" | ... 29 more ... | "useFileSuffix">, "strip", ZodTypeAny, { ...; }, { ...; }> | ... 24 more ... | ZodObject<...>' is not assignable to parameter of type 'AnyZodObject'.

@jakebailey
Copy link
Member Author

Oh wow, that's it?

@Benjamin-Dobell
Copy link

It's worth noting the top 800 projects almost certainly aren't representative of real world use. They're almost exclusively going to be libraries. It's applications that require this functionality when they include multiple interoperating libraries that share transitive dependencies.

@jakebailey
Copy link
Member Author

Okay. Can you provide an example?

@jakebailey
Copy link
Member Author

(Or, go try https://www.npmjs.com/package/@typescript/native-preview and see if it already works for you?)

@Benjamin-Dobell
Copy link

Benjamin-Dobell commented Oct 28, 2025

I already included a minimal reproduction at microsoft/typescript-go#1906. Speaking of deduplication, GitHub doesn't do a great job of retaining information when deduplicating issues 😅

The problem here is Kysely is a toolkit made up of many interoperating libraries, and they have private fields. This of course wouldn't be an issue if interfaces were used in the APIs rather than classes.

@jakebailey
Copy link
Member Author

But aren't the errors it states about duplication objectively correct, and the code just happens to be lucky to not notice internally?

@Benjamin-Dobell
Copy link

Benjamin-Dobell commented Oct 28, 2025

I wouldn't say it's objectively correct. It's unfortunately more complex than that. It depends on whether:

  • The build process deduplicates the modules.
  • OR, whether the type is used exclusively as type rather than instantiation / runtime variable use i.e. used like import type rather than import.

After all, the API wants a Kysely object, specifically one from the kysely module v0.28.8. That is what is being provided. If the API uses this object (or rather the kysely module as a whole) exclusively as a type, then this is sound.

It's unsound if there's no deduplication during the build process AND the kysely module is being used not simply as a type. That'd potentially cause issues if the package instantiates a Kysely and takes an external Kysely and they both reference some static/module level state.

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