Don't propagate partial union/intersection properties between caches #58083
Conversation
This file contains 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
typescript-bot
added
Author: Team
For Uncommitted Bug
|
@typescript-bot test it |
|
Hey @ahejlsberg, the results of running the DT tests are ready. Everything looks the same! |
|
@ahejlsberg Here are the results of running the user tests comparing Everything looks good! |
|
@ahejlsberg Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@ahejlsberg Here are the results of running the top 400 repos comparing Everything looks good! |
|
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 "";
} |
|
@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'
} |
|
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 😅 |
|
Maybe add each to both a compiler test and to a fourslash test? |
|
I've also had success testing these sorts of things via unit tests, e.g. |
|
I think it would be good to cherry-pick this into 5.4.5. |
|
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 @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. |
|
@typescript-bot pack this |
|
Hey @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 |
|
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
added
For Milestone Bug
|
@typescript-bot cherry-pick this to release-5.4 |
jakebailey
approved these changes
Apr 9, 2024
|
Hey, @jakebailey! I've created #58136 for you. This involved updating baselines; please check the diff. |
DanielRosenwasser
pushed a commit
that referenced
this pull request
Apr 9, 2024
…e-5.4 (#58136) Co-authored-by: Anders Hejlsberg <andersh@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.