Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intersection between separate types with getter and setter does not allow assignment to property #45122

Closed
bwrrp opened this issue Jul 20, 2021 · 4 comments · Fixed by #45263
Closed
Labels
Bug A bug in TypeScript Help Wanted You can do this
Milestone

Comments

@bwrrp
Copy link
Contributor

bwrrp commented Jul 20, 2021

Bug Report

I'm trying to define a property that is writable in some cases and readonly in others, by defining its getter and setter in different types and using the intersection between those when the property should be writable. However, TypeScript still complains about the property not being writable.

🔎 Search Terms

intersection getter setter readonly

🕗 Version & Regression Information

Reproducible on the playground with both the latest stable (4.3.5) as well as with 4.4.0 beta and the 'nightly' option.

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about getter / setter / readonly / intersection types

⏯ Playground Link

Playground link with relevant code

Alternative attempt using readonly

💻 Code

I've tried the following approaches - using a getter and setter:

type T = {get prop(): string;} & {set prop(v: string)};
function aaa(a: T) {
  a.prop = 'aaa';
}

and also using the readonly keyword:

type T = {readonly prop: string;} & {prop: string};
function aaa(a: T) {
  a.prop = 'aaa';
}

🙁 Actual behavior

In both cases the assignment is not allowed by the compiler, and I got a Cannot assign to 'prop' because it is a read-only property. error.

🙂 Expected behavior

I expected A & B to behave as "This type implements both A and B at the same time". Because of that I would have expected a readonly property defined in A to be writable if B provides a setter.

@jcalz
Copy link
Contributor

jcalz commented Jul 21, 2021

I think I agree.

My intuition is that readonly means "we don't know if this can be written or not", and not "this definitely cannot be written". That's what it means with readonly any[] compared to any[], and why you can assign any[] to readonly any[] but not vice versa. A readonly any[] may indeed have a push() method, but you can't safely call it.

If that intuition holds for readonly properties, then I'd expect unions to be readonly if any of the members are, so {readonly p: any} | {p: any} would effectively yield {readonly p: any}. And this does happen:

type U = Pick<{ readonly p: any } | { p: any }, "p">
// type U = { readonly p: any; } 🙂

I'd also expect intersections to not be readonly unless all of the members are, so {readonly p: any} & {p: any} would effectively yield {p: any}, yet this is not what happens:

type I = Pick<{ readonly p: any } & { p: any }, "p">
// type I = { readonly p: any; } 🙁

Oops. readonly seems to be "infectious" in a similar way to any. And readonly is intentionally unsound in other ways, as discussed in #13347. So for all I know there's a good reason why the intersection behaves the way it does now. Does anyone have an insight here? And how much would it break if we changed the behavior?

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Jul 21, 2021
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jul 21, 2021
@RyanCavanaugh RyanCavanaugh added the Help Wanted You can do this label Jul 21, 2021
@RyanCavanaugh
Copy link
Member

So for all I know there's a good reason why the intersection behaves the way it does now. Does anyone have an insight here? And how much would it break if we changed the behavior?

This (the OP example) only even became legal in 4.3; it's an oversight that this particular combination wasn't made to work. The definition of intersection implies that this should be legal.

bwrrp added a commit to bwrrp/TypeScript that referenced this issue Jul 23, 2021
@bwrrp
Copy link
Contributor Author

bwrrp commented Jul 24, 2021

While checking this change against the existing unit tests, I came across a change in behavior in the DOM typings I'm not sure about.

The defaultView property of the Document interface is typed as (WindowProxy & typeof globalThis) | null. There are a number of props that are defined as readonly on WindowProxy (which is actually interface Window), but writable on globalThis (e.g., readonly navigator: Navigator on Window vs. declare var navigator: Navigator for the global). Before this change, those properties of the defaultView would be readonly, but afterwards they are now writable.

I'm not sure what the idea is behind using an intersection type here instead of defining any missing globals as Window properties? Given that window.document.defaultView === globalThis it seems odd that some of these properties would be writable by global assignment but not through this property.

@sandersn
Copy link
Member

The defaultView definition comes from microsoft/TypeScript-DOM-lib-generator#820. It's basically copying microsoft/TypeScript-DOM-lib-generator#715, which did the same thing for Window & typeof globalThis. There, the intention was to include global properties on window via typeof globalThis, but also any properties defined via interface merging with multiple declarations of interface Window {. After TS 3.3, which introduced globalThis, we expect most people to declare globals and be able to use them on window without the interface merging trick.

Like most intersections in the real world, properties appearing in both sides are unfortunate and hopefully identical.

In this case, I think it's fine to lose the readonly-ness of the Window property. The readonly would be nice but I think it's not surprising, for example, that assigning to navigator won't actually do anything.

bwrrp added a commit to bwrrp/TypeScript that referenced this issue Aug 22, 2021
bwrrp added a commit to bwrrp/TypeScript that referenced this issue Aug 25, 2021
BobobUnicorn pushed a commit to BobobUnicorn/TypeScript that referenced this issue Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants