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
chore: switch to single version policy for typescript #16592
chore: switch to single version policy for typescript #16592
Conversation
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 7908313:
|
Perf AnalysisNo significant results to display. All results
Perf Analysis (Fluent)Perf comparison
Perf tests with no regressions
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 55194b54de8fe4e3b9aef0395321bec1d9cbc2c8 (build) |
Before we modify the TS version for web-components we'll need to talk to @chrisdholt. |
it's not modified, just set to exact version (which is used anyways via |
ed227b5
to
2de38f0
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.
LGTM for v0
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.
This looks good to me once the issue with react-monaco-editor is fixed.
Also once this is merged it should fix #14371.
...ge/@fluentui-react-monaco-editor-2021-01-22-19-28-04-hotell-build-system-ts-migration-1.json
Outdated
Show resolved
Hide resolved
2de38f0
to
49b1546
Compare
On the web-components topic. Am I correct to assume that web-components is not impacted by the single version policy? Meaning we can change the typescript version web-components uses independent of react. |
@@ -92,7 +92,7 @@ | |||
"ts-loader": "^7.0.2", | |||
"ts-node": "^8.0.0", | |||
"tsconfig-paths": "^3.9.0", | |||
"typescript": "^3.9.0", | |||
"typescript": "3.9.7", |
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.
This seems fine - in the future anything which is intended to fix a React problem should likely not require a change here. There's no dependency on React for these packages and so I'd expect changes to be scoped to only those necessary.
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.
no worries, this has nothing to do with React. It's a 1st step to start using single version policy (and exact versions) for devDeps with a possibility to opt out. I could left web-components part of the repo as is but to prevent any kinds of version confusion/mismatches I set the version to be explicit.
…l-build-system-ts-migration-1.json Co-authored-by: Elizabeth Craig <ecraig12345@gmail.com>
5990afe
to
7908313
Compare
@Hotell looks like we have sufficient sign off, if you are good to go I'll merge this (P.S. for a smaller change I would probably just merge it rather than asking). |
I can merge it (admin rights), although that makes me quite uncomfortable heh |
In cases like this we tend to go ahead and admin merge. Especially since github makes it very hard to tell which approver is "missing" and all the most relevant people for this type of change have already approved. |
@Hotell yeah, I agree I would feel uncomfortable admin merging my own PR, that's why I offered to do it 😄 . I would normally just merge it right now, but with a big change like this I'll let you merge it when you're awake 👍 In this case, I would recommend merging this, and not trying to hunt down sign off from every single codeowner |
🎉 Handy links: |
Co-authored-by: Elizabeth Craig <ecraig12345@gmail.com>
Pull request checklist
$ yarn change
Description of changes
(give an overview)
Focus areas to test
(optional)