chore: upgrade typescript to 5.5.2 #33498
Open
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
fabricteam
reviewed
Dec 20, 2024
mainframev
commented
Dec 20, 2024
| @@ -85,7 +91,9 @@ export function classNamesFunction<TStyleProps extends {}, TStyleSet extends ISt | |||
| typeof styleFunctionOrObject === 'function' && | |||
| (styleFunctionOrObject as StyleFunction<TStyleProps, TStyleSet>).__noStyleOverride__ | |||
| ) { | |||
| return styleFunctionOrObject(styleProps) as IProcessedStyleSet<TStyleSet>; | |||
| return (styleFunctionOrObject as IStyleFunction<TStyleProps, TStyleSet>)( | |||
There was a problem hiding this comment.
had to add additional casting, throwing after upgrade with this TS2349:
There was a problem hiding this comment.
| state.components?.[slotName] === undefined || typeof state.components[slotName] === 'string' | ||
| ? asProp || state.components?.[slotName] || 'div' | ||
| : state.components[slotName] | ||
| ) as React.ElementType<R[K]>; | ||
| : state.components[slotName]; |
There was a problem hiding this comment.
after upgrade throws TS2590: Expression produces a union type that is too complex to represent
There was a problem hiding this comment.
that's strange, there shouldn't be difference between asserting on definition vs this artificial alias you created. can you double check please ?
There was a problem hiding this comment.
I think there is a difference for how TS is processing it, it's too complex for TS to handle assertion context in the first case, as it's attempting to make a full type validation against of a both ternary branches 🤷🏻
Smth like this should also work:
const slot: React.ElementType<R[K]> =
state.components?.[slotName] === undefined || typeof state.components[slotName] === 'string'
? ((asProp || state.components?.[slotName] || 'div') as React.ElementType<R[K]>)
: (state.components[slotName] as React.ElementType<R[K]>);There was a problem hiding this comment.
there are various false positives in this implementation starting with return type readonly [React.ElementType<R[K]> | null, R[K]], but what is being done inside is casting the slot to React.ElementType<R[K]> instead of explicit union with null.
anyways...
the slot variable instantiation is so overly complex that it is impossible to capture a proper type from all these inline short-circuit checks
at this stage we should do following.
const slot = (state.components?.[slotName] === undefined || typeof state.components[slotName] === 'string'
? asProp || state.components[slotName] || 'div'
: state.components[slotName]) as unknown as React.ElementType<R[K]>;- no extra JS runtime
- less complex for TSC to figure out things
- better express the intent as the result expression is truly unknown :)
mainframev
force-pushed
the
chore/ts-update-5-5
branch
2 times, most recently
from
29cadea to
ea576b0
Compare
mainframev
commented
Dec 20, 2024
mainframev
force-pushed
the
chore/ts-update-5-5
branch
from
ea576b0 to
3ee9cbd
Compare
|
Pull request demo site: URL |
mainframev
force-pushed
the
chore/ts-update-5-5
branch
from
3ee9cbd to
83e4110
Compare
📊 Bundle size report
Unchanged fixtures
|
mainframev
force-pushed
the
chore/ts-update-5-5
branch
2 times, most recently
from
0e865c5 to
0a3b8eb
Compare
mainframev
force-pushed
the
chore/ts-update-5-5
branch
3 times, most recently
from
9f52366 to
415da9a
Compare
mainframev
force-pushed
the
chore/ts-update-5-5
branch
from
415da9a to
eccd0d4
Compare
mainframev
commented
Dec 27, 2024
mainframev
requested review from
a team,
Mitch-At-Work,
mltejera,
marcosmoura,
GeoffCoxMSFT,
chpalac and
ling1726
as code owners
Hotell
reviewed
Jan 17, 2025
| @@ -15,7 +15,9 @@ export function concatStyleSetsWithProps<TStyleProps, TStyleSet extends IStyleSe | |||
| const result: Array<DeepPartial<TStyleSet>> = []; | |||
| for (const styles of allStyles) { | |||
| if (styles) { | |||
| result.push(typeof styles === 'function' ? styles(styleProps) : styles); | |||
| result.push( | |||
| typeof styles === 'function' ? (styles as IStyleFunction<TStyleProps, TStyleSet>)(styleProps) : styles, | |||
There was a problem hiding this comment.
checked this a bit more.
because the IStyleFunctionOrObject type is very complex ( including recursion ), the typeof function check is not enough to narrow down the right value from the union.
Cleaner solution is to introduce custom type guard to help TS navigate in these non trivial waters ( might lead also to faster tsc )
export function isStyleFunction<TStylesProps, TStyleSet extends IStyleSetBase>(
val: IStyleFunctionOrObject<TStylesProps, TStyleSet>,
): val is IStyleFunction<TStylesProps, TStyleSet> {
return typeof val === 'function';
}⬇️
result.push(isStyleFunction(styles) ? styles(styleProps) : styles);no errors ✅.
similar approach can be used also in other related changes you made as part of this PR.
chore(utilities): update api.md chore(react-utilities): update api.md chore(style-utilities): update api.md
mainframev
force-pushed
the
chore/ts-update-5-5
branch
from
4f245a3 to
701f5a5
Compare
🕵 FluentUIV0 No visual regressions between this PR and main |
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.
@importJSDoc tags.Release notes
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-4.html
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-5.html
Related to configDir template var: the initial assumption, that it will help to get rid of explicit typeRoots for
tsconfig.cy.jsonfiles appeared to be wrong. It works for scenarios, where derived config (packages//tsconfig.json) has to have paths related to final directory, before it was needed to re-specify the config property for derived config.Previous Behavior
5.3.3
New Behavior
5.5.2
Related Issue(s)
(nx run-many -t build type-check --skip-nx-cache)