Don't propagate partial union/intersection properties between caches - #58083
Merged
Conversation
TypeScript Bot (typescript-bot)
added
Author: Team
For Uncommitted Bug
Member
Author
|
TypeScript Bot (@typescript-bot) test it |
Contributor
Contributor
|
Hey Anders Hejlsberg (@ahejlsberg), the results of running the DT tests are ready. Everything looks the same! |
Contributor
|
Anders Hejlsberg (@ahejlsberg) Here are the results of running the user tests comparing Everything looks good! |
Contributor
|
Anders Hejlsberg (@ahejlsberg) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
|
Anders Hejlsberg (@ahejlsberg) Here are the results of running the top 400 repos comparing Everything looks good! |
Contributor
|
minimal repro case interface ComponentOptions<Props> {
setup?: (props: Props) => void;
name?: string;
}
interface FunctionalComponent<P> {
(props: P): void;
}
type ConcreteComponent<Props> =
| ComponentOptions<Props>
| FunctionalComponent<Props>;
type Component<Props = {}> = ConcreteComponent<Props>;
type WithInstallPlugin = { _prefix?: string };
export function withInstall<C extends Component, T extends WithInstallPlugin>(
component: C | C[],
target?: T,
): string {
const componentWithInstall = (target ?? component) as T;
const components = Array.isArray(component) ? component : [component];
const { name } = components[0];
if (name) {
return name;
}
return "";
} |
Member
Author
|
Mateusz Burzyński (@Andarist) It is quite odd. Your example reproduces with the command-line compiler, but not in the IDE (VS Code). Meanwhile, this even shorter example reproduces in the IDE, but not with the command-line compiler: type Foo = { x: number } | { toString: string | undefined };
function gg<T extends Foo>(x: T & { toString(): string | undefined }, foo: Foo) {
foo.toString; // Error, Property 'toString' does not exist on type 'Foo'
} |
Contributor
|
I was experiencing what I would assume to be out-of-order checking in the IDE so at some point I pasted the already slimmed down repro into the playground and continued there. Since it was a single file I could test it out quite reliably there - I didn't even run this with
This is very strange 😅 |
Member
|
Maybe add each to both a compiler test and to a fourslash test? |
Member
|
I've also had success testing these sorts of things via unit tests, e.g. |
Member
|
I think it would be good to cherry-pick this into 5.4.5. |
Member
|
Here's a fourslash test based on #58083 (comment) which fails before this PR but passes after: /// <reference path="fourslash.ts" />
// @strict: true
// @lib: esnext
//// interface ComponentOptions<Props> {
//// setup?: (props: Props) => void;
//// name?: string;
//// }
////
//// interface FunctionalComponent<P> {
//// (props: P): void;
//// }
////
//// type ConcreteComponent<Props> =
//// | ComponentOptions<Props>
//// | FunctionalComponent<Props>;
////
//// type Component<Props = {}> = ConcreteComponent<Props>;
////
//// type WithInstallPlugin = { _prefix?: string };
////
////
//// /**/
//// export function withInstall<C extends Component, T extends WithInstallPlugin>(
//// component: C | C[],
//// target?: T,
//// ): string {
//// const componentWithInstall = (target ?? component) as T;
//// const components = Array.isArray(component) ? component : [component];
////
//// const { name } = components[0];
//// if (name) {
//// return name;
//// }
////
//// return "";
//// }
verify.noErrors();
goTo.marker();
edit.insert("type C = Component['name']");
verify.noErrors();I'm observing the opposite of what Anders Hejlsberg (@ahejlsberg) is seeing, in that the above code fails in the editor for me but there's no errors when made into a compiler test. But I think it matches the original issue well enough to be a test for this PR, though. |
Member
|
TypeScript Bot (@typescript-bot) pack this |
Contributor
Contributor
|
Hey Jake Bailey (@jakebailey), I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
Member
|
Confirmed that this test works just like the original issue in the playground; will push here since we want the patch out very shortly. |
TypeScript Bot (typescript-bot)
added
For Milestone Bug
Member
|
TypeScript Bot (@typescript-bot) cherry-pick this to release-5.4 |
Contributor
Jake Bailey (jakebailey)
approved these changes
Apr 9, 2024
Contributor
|
Hey, Jake Bailey (@jakebailey)! I've created #58136 for you. This involved updating baselines; please check the diff. |
Daniel Rosenwasser (DanielRosenwasser)
pushed a commit
that referenced
this pull request
Apr 9, 2024
…e-5.4 (#58136) Co-authored-by: Anders Hejlsberg <andersh@microsoft.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There's currently no repro in this PR since it is really hard to reproduce the conditions in a stand-alone manner.
Fixes #58050.