Repository navigation
Some Playwright types now reported as any after upgrading to 5.7.2 #60619
Description
🔎 Search Terms
I manually scrolled through all issues reported since the release of 5.7.2.
🕗 Version & Regression Information
- This changed between versions 5.6.3 and 5.7.2
⏯ Playground Link
https://github.com/mrmckeb/pw-repro
💻 Code
Minimal repro available here:
https://github.com/mrmckeb/pw-repro
The Playwright team are also able to reproduce:
microsoft/playwright#33763
The affected code (from the repro) is:
import { test as base, expect } from "@playwright/test";
const test = base.extend({
// baseUrl, page, use are now `any`
page: async ({ baseURL, page }, use) => {
await page.goto(baseURL);
await use(page);
},
});🙁 Actual behavior
Types that were previously available are now typed as any. JSDoc continues to work as expected.
🙂 Expected behavior
Types behave as they did in TypeScript 5.6.3.
Additional information about the issue
We found this issue when upgrading from 5.6.3 to 5.7.2. I couldn't find a related issue here, so raised it at the Playwright repo to at least verify that they could reproduce and that they didn't think it was an issue with their types.
The Playwright team also confirmed that they have 71 new errors across 41 files when moving to TypeScript 5.7.2.
Activity
Andarist commentedon Nov 26, 2024
bisects to #52095 , minimal-ish repro case: TS playground
I already have a somewhat good grasp of the issue. I'll write more about it tomorrow though and continue the investigation.
Andarist commentedon Nov 27, 2024
I don't understand why
Exclude<R, Function>is there but I can also see how it could potentially mess things up (although I wasn't yet able internally verify why it messes it up). Unless I'm missing some constraint that those types try to apply there I think this is actually incorrect.I'll dive deeper into it later but in the meantime I've also created a different repro case that doesn't include that
Excludeand I think it's better to focus on it first: TS playgrounddgozman commentedon Nov 27, 2024
Mateusz Burzyński (@Andarist) The
Exclude<R, Function>construct is to allow the fixture value in a literal form, e.g.page: 'foo', in addition to the fixture function likepage: async ({}, use) => use('foo'). However, if the fixture typeFis a function, the idea is to only allow the latter form, to avoid any possibility of the confusion betweenFandasync ({}, use) => {...}.By the way, I once tried to replace
[K in keyof T]?:with[K in Exclude<keyof T, keyof PT>]?:to avoid the clash betweenTandPTproperties. However, that breaks whenRorExclude<R, Function>is allowed in theTestFixturetype, because it infersTinextend<T extends KeyValue>asKeyValueand every property becomesany.Andarist commentedon Nov 27, 2024
Ah ye, I already figured this part out (side note - you don't have any type tests in Playwright for this scenario, no test fail after replacing
Exclude<R, Function>with justRthere).I'm not yet sure where it breaks internally but, in general, "not a type" isn't something greatly supported by TS. I put together some notes~ on various ways to do it in this TS playground. Ultimately, only your strategy gives the desired results here.
You are on the right track here when it comes to the root cause. The problem is that your
PTis already a resolved mapped type with function properties. But thenTis being inferred into from the same object that is typed using an intersection over mapped types applied to T and PT and it really doesn't know that it shouldn't usepageproperty because it's already part ofPT.With my change the compiler uses all property types that could come out of deferred instantiations at this position. So it uses function types from all of those 4 mapped types and they are just not compatible with each other. That's how we end up with implicit anys.
To be fair, even before TS 5.7 the inference didn't quite work OK in your case (the contextual parameter types did though): TS playground. I somewhat doubt you care about it, when defining extra fixtures it's advised to use type params explicitly here instead of relying on the inference. It would certainly be great to make it work though.
Luckily-ish since TS 5.6 (since this PR) we can leverage filtering mapped types to disambiguite slightly between those mapped types: TS playground
And when it comes to the inference, the situation could be improved by this PR. We can see some improvements already on the TS playground using a TS build from that PR: TS playground. Based on those findings, I need to take a look at what happens with context-sensitive functions there because they seem to brak this inference.
Note that on the TS playground above I also replaced multiple
extends KeyValuewithextends {}. This is a safer default because TS uses the constraint when there are no inference candidates. Throughextends KeyValueanys can easily creep into the user types and I think it's best to avoid that risk.TLDR: I created a PR with a fix for this in Playwright here