fix: add loose auto complete to string literals where applicable #966
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
janglad
changed the title
|
@J0 just checking in if you could have a look at this 🙏. Maybe this change is unwanted but either way it should be a relatively quick review and it would make my life a tiny bit nicer 😄 |
J0
changed the title
|
@janglad Thanks for the PR - could we trouble you to point us to the relevant TypeScript docs for this (if any)? |
I'm sadly not aware of any official documentation for this. It it is however something that's used in quite a few big libraries (like the types for React). FWIW TS tests agains Definitely Typed and very rarely does breaking changes, I would personally not worry that this breaks in the future. |
J0
approved these changes
Nov 14, 2024
|
Thank you! |
|
Yay, thanks! |
kangmingtay
pushed a commit
that referenced
this pull request
Dec 4, 2024
🤖 I have created a release *beep* *boop* --- ## [2.66.1](v2.66.0...v2.66.1) (2024-12-04) ### Bug Fixes * add loose auto complete to string literals where applicable ([#966](#966)) ([fd9248d](fd9248d)) * add new error codes ([#979](#979)) ([dfb40d2](dfb40d2)) * don't remove session for identity linking errors ([#987](#987)) ([e68ebe6](e68ebe6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.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.
What kind of change does this PR introduce?
Add "loose autocomplete" to
WeakPasswordReasons,AMREntry.method,Factor.factor_type,SignInWithIdTokenCredentials.providerandAuthError.codeWhat is the current behavior?
The above types define possible string literal values, which is great, but they're unioned with
string. As a regular"literal" | stringjust ends up as astringthis foregoes the benefit of defining the literals in the first place.What is the new behavior?
TypeScript still allows any possible string as a valid value (notice no error on the "not in there"), but now the predefined literals show up as hints. This is done by using
"literal" | string & {}. While this seems hacky, it's something that works and something the TS team test against meaning they won't "fix" it in future releases.Currently these hints aren't shown as the type just ends up as a simple
string.Additional context
I believe this can be quite helpful for people when trying to handle errors etc. I know
string & {}might look somewhat weird for maintainers, if preferable I can create aLooseAutocomplete<T>or something and wrap them in there but this seemed cleaner.