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

Allow satisfies keyof assertions in computed property names #58829

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

weswigham
Copy link
Member

Fixes #58800

This implements the proposal in the linked issue, along with the addendum about invalid satisfies keyof property types still getting unique, singular property names (which ensures declaration file output always includes the computed names, even when they don't resolve).

We can still bikeshed the syntax if anyone really wants, but I think this probably fits the bill without actually introducing anything too far off of what we already allow. Though yes, 15 characters is a lot - that's satisfies for you.

The open question I think worth answering is if satisfies keyof should be allowed in all expression positions and not just in computed property names. It's certainly at least useful as a shorthand for satisfies (string | number | symbol) (though the check should be a wee bit stronger than that, since it also checks unit-ness), which has a similar effect on literals to as const, without readonly-ing or const-ing array/object types (and instead erroring on them). It's probably worth noting, while generics are generally forbidden, given T extends "a" | "b", T & "a" simplifies to "a" under current rules, so presents as a unit type - making it the only generic form, afaik, that can pass the unit-type-key check.

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jun 11, 2024
@typescript-bot
Copy link
Collaborator

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

@weswigham
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 11, 2024

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

Command Status Results
pack this ✅ Started ✅ Results

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 11, 2024

Hey @weswigham, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/162167/artifacts?artifactName=tgz&fileId=3CD46A7AA8AD3812EBDD66F53B563E5CC213528DD504920028F176DBB3DC40A502&fileName=/typescript-5.6.0-insiders.20240611.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.6.0-pr-58829-3".;

@fatcerberus
Copy link

Though yes, 15 characters is a lot - that's satisfies for you.

That's not actually my main complaint, it's a more general concern than that. satisfies is documented as not affecting the types of expressions (except via contextual typing, which people already complain about)--it's just a type-checking mechanism--so through that lens, { [x]: "foo" } and { [x satisfies keyof]: "foo" } should be completely equivalent assuming they both typecheck - and yet they may produce completely semantically different declarations!

tl;dr This feels like a confusing overloading of the satisfies keyword.

The open question I think worth answering is if satisfies keyof should be allowed in all expression positions and not just in computed property names. It's certainly at least useful as a shorthand for satisfies (string | number | symbol)

If you do go that route, then I'd suggest to go all the way and just make keyof with no argument basically an alias for PropertyKey.

@lazytype
Copy link

lazytype commented Jun 20, 2024

I think logically satisfies keyof makes more sense, but aesthetically I think I'd be happy with re-fitting the deferred prefix operator introduced in #58729 if it goes through.

class Foo {
  [deferred x]: ...;
}
vs
class Foo {
  [x satisfies keyof]: ...;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Status: Waiting on reviewers
4 participants