chore: enable proper type checking within all just.config.ts and its dependents#25852
Merged
Hotell merged 6 commits intoDec 23, 2022
Merged
Conversation
Hotell
changed the title
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 10c20e1:
|
Collaborator
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 1650 | 1681 | 5000 | |
| Button | mount | 1182 | 1203 | 5000 | |
| FluentProvider | mount | 1979 | 1989 | 5000 | |
| FluentProviderWithTheme | mount | 798 | 784 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 750 | 725 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 788 | 789 | 10 | |
| MakeStyles | mount | 2314 | 2323 | 50000 | |
| Persona | mount | 3787 | 3691 | 5000 | |
| SpinButton | mount | 3179 | 3202 | 5000 |
Collaborator
🕵 fluentuiv8 Open the Visual Regressions report to inspect the 2 screenshots✅ There was 0 screenshots added, 2 screenshots removed, 1043 screenshots unchanged, 0 screenshots with different dimensions and 0 screenshots with visible difference. unknown 2 screenshots
|
Collaborator
Perf Analysis (
|
Collaborator
🕵 fluentuiv9 No visual regressions between this PR and main |
Collaborator
📊 Bundle size reportUnchanged fixtures
|
Collaborator
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| BaseButton | mount | 1151 | 1154 | 5000 | |
| Breadcrumb | mount | 2698 | 2728 | 1000 | |
| Checkbox | mount | 2566 | 2456 | 5000 | |
| CheckboxBase | mount | 2332 | 2242 | 5000 | |
| ChoiceGroup | mount | 4334 | 4428 | 5000 | |
| ComboBox | mount | 1166 | 1241 | 1000 | |
| CommandBar | mount | 9073 | 9070 | 1000 | |
| ContextualMenu | mount | 11267 | 11032 | 1000 | |
| DefaultButton | mount | 1303 | 1378 | 5000 | |
| DetailsRow | mount | 3313 | 3541 | 5000 | |
| DetailsRowFast | mount | 3601 | 3537 | 5000 | |
| DetailsRowNoStyles | mount | 3409 | 3326 | 5000 | |
| Dialog | mount | 2923 | 3001 | 1000 | |
| DocumentCardTitle | mount | 561 | 559 | 1000 | |
| Dropdown | mount | 3034 | 3062 | 5000 | |
| FocusTrapZone | mount | 1905 | 1923 | 5000 | |
| FocusZone | mount | 1908 | 1875 | 5000 | |
| GroupedList | mount | 1788 | 2018 | 2 | |
| GroupedList | virtual-rerender | 1068 | 1075 | 2 | |
| GroupedList | virtual-rerender-with-unmount | 1554 | 1639 | 2 | |
| GroupedListV2 | mount | 546 | 543 | 2 | |
| GroupedListV2 | virtual-rerender | 548 | 525 | 2 | |
| GroupedListV2 | virtual-rerender-with-unmount | 573 | 504 | 2 | |
| IconButton | mount | 1896 | 1892 | 5000 | |
| Label | mount | 719 | 685 | 5000 | |
| Layer | mount | 4173 | 4127 | 5000 | |
| Link | mount | 782 | 765 | 5000 | |
| MenuButton | mount | 1545 | 1523 | 5000 | |
| MessageBar | mount | 2336 | 2097 | 5000 | |
| Nav | mount | 3076 | 3023 | 1000 | |
| OverflowSet | mount | 1338 | 1318 | 5000 | |
| Panel | mount | 2485 | 2509 | 1000 | |
| Persona | mount | 1204 | 1277 | 1000 | |
| Pivot | mount | 1659 | 1575 | 1000 | |
| PrimaryButton | mount | 1417 | 1477 | 5000 | |
| Rating | mount | 6810 | 6619 | 5000 | |
| SearchBox | mount | 1403 | 1350 | 5000 | |
| Shimmer | mount | 2600 | 2760 | 5000 | |
| Slider | mount | 1940 | 1949 | 5000 | |
| SpinButton | mount | 4200 | 4308 | 5000 | |
| Spinner | mount | 789 | 728 | 5000 | |
| SplitButton | mount | 2910 | 2895 | 5000 | |
| Stack | mount | 770 | 828 | 5000 | |
| StackWithIntrinsicChildren | mount | 2130 | 2191 | 5000 | |
| StackWithTextChildren | mount | 4574 | 4572 | 5000 | |
| SwatchColorPicker | mount | 9876 | 9837 | 5000 | |
| TagPicker | mount | 2604 | 2420 | 5000 | |
| TeachingBubble | mount | 83925 | 82883 | 5000 | |
| Text | mount | 710 | 697 | 5000 | |
| TextField | mount | 1601 | 1556 | 5000 | |
| ThemeProvider | mount | 1405 | 1544 | 5000 | |
| ThemeProvider | virtual-rerender | 1042 | 1033 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 2036 | 2071 | 5000 | |
| Toggle | mount | 1110 | 1101 | 5000 | |
| buttonNative | mount | 551 | 534 | 5000 |
Hotell
force-pushed
the
hotell/build/scripts/ts-solution-config-v7
branch
from
d651a46 to
2fa953a
Compare
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 0ba1e7e3d1804a407efd9b58fdb38318f1d99447 (build) |
Collaborator
🕵 FluentUI-v0 No visual regressions between this PR and main |
Hotell
force-pushed
the
hotell/build/scripts/ts-solution-config-v7
branch
from
877bced to
a125a6b
Compare
Hotell
commented
Dec 21, 2022
|
|
||
| // copied from scripts/just.config.js with addition of generate-json | ||
| task('build', series('clean', 'copy', 'sass', 'generate-json', 'ts')).cached(); | ||
| task('build', series('clean', 'copy', 'sass', 'generate-json', 'ts')).cached!(); |
Contributor
Author
There was a problem hiding this comment.
whilst using ! is deprecated and should be avoided, this is caused by bad just types, so sticking with ! instead of nodejs standard ?
| @@ -1,4 +1,6 @@ | |||
| import { preset, task, resolveCwd } from '@fluentui/scripts'; | |||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | |||
| // @ts-ignore - parallel-webpack has no types | |||
Contributor
Author
There was a problem hiding this comment.
I could write types manually for this package but I decided to not do so ATM (its the only usage in whole repo)
| @@ -1,4 +1,8 @@ | |||
| { | |||
| "extends": "@tsconfig/node14/tsconfig.json", | |||
| "include": ["./{apps,packages}/*/just.config.ts"] | |||
Contributor
Author
There was a problem hiding this comment.
this never worked (bash expansion)
Hotell
requested review from
a team,
GeoffCoxMSFT,
Jacqueline-ms and
robtaft-ms
as code owners
TristanWatanabe
approved these changes
Dec 21, 2022
spmonahan
approved these changes
Dec 21, 2022
khmakoto
approved these changes
Dec 22, 2022
ling1726
reviewed
Dec 22, 2022
|
|
||
| while (packagesToResolve.length) { | ||
| const pkg = packagesToResolve.shift(); | ||
| const pkg = packagesToResolve.shift() as string; |
Contributor
There was a problem hiding this comment.
why is this cast needed? typescript should know that it's a string from line 9
Contributor
Author
There was a problem hiding this comment.
shift() returns T or undefined if array is empty, which in our case will never happen. but with existing construct TS cannot understand that that situation never happens, thus the assertion (which will not throw runtime error)
Contributor
Author
There was a problem hiding this comment.
refactored the code to contain less assertions 10c20e1
ling1726
approved these changes
Dec 22, 2022
…and their dependencies
Contributor
Author
|
admin merged with all checks passing as got approvals from all core teams |
q1b
pushed a commit
to q1b/fluentui
that referenced
this pull request
Jan 24, 2023
…dependents (microsoft#25852) * chore: enable strict checking in all just-config files and their dependencies * ci: enable type checking of just.configs * generate change files
Hotell
added a commit
to Hotell/fluentui
that referenced
this pull request
Feb 9, 2023
…dependents (microsoft#25852) * chore: enable strict checking in all just-config files and their dependencies * ci: enable type checking of just.configs * generate change files
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Pre-requirement:
Previous Behavior
just.config.tswith all its dep tree is not giving proper DX/is not validated against anythingNew Behavior
Related Issue(s)
/scripts) #24349