-
Notifications
You must be signed in to change notification settings - Fork 15
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
improve getter tools, don't export optional getters #1574
Conversation
🦋 Changeset detectedLatest commit: cdd01b0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
6a2176b
to
75f504b
Compare
75f504b
to
d501174
Compare
Visit the preview URL for this PR (updated for commit cdd01b0): https://penumbra-ui-preview--pr1574-turbocrime-getter-re-vys5cpxm.web.app (expires Tue, 10 Sep 2024 22:30:28 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 709d729610ef7a6369b23f1cb2b820a60cc685b1 |
bc5bfb2
to
d4fea52
Compare
other reviewers would be helpful to sanity check |
b1ba4f9
to
cdd01b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rebased
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GetterMissingValueError
uses es2022 'error cause' like other parts of the repository. error cause was previously permitted by this tsconfig, but for some reason downgraded to es2020 at some point.
the many other fields added by a8a5f41 were already present in the configs which are extended, so all of those added fields had no effect.
so it's reverted essentially to abcaab6, plus a newer lib. i can't identify a practical difference aside from the lib option.
Lines 1 to 10 in abcaab6
{ | |
"compilerOptions": { | |
"composite": true, | |
"exactOptionalPropertyTypes": false, | |
"noEmit": true, | |
"target": "ESNext" | |
}, | |
"extends": ["@tsconfig/strictest/tsconfig.json", "@tsconfig/vite-react/tsconfig.json"], | |
"include": ["components", "lib", "src", "tests-setup.ts"] | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great DX improvement 👍
This reverts commit a7ed0ab.
improves legibility of computed result of the
Getter
typeundefined
as part of the target type, when the target type includes undefinedNonNullable
as part of the target type, when the target type does not include undefinedgetters package no longer exports optional getters. exporting optional getters has the potential to cause confusion. a consumer who wishes to use an optional getter can use the
.optional()
utility provided on every getter..optional()
can now be applied at any point in a getter pipe, which affects the pipe in an intuitive way.optional()
call will continue to assert results.optional()
call may return undefined