Skip to content
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

Bump TypeScript in template from 4.8.4 to 5.0.4 #36862

Closed
wants to merge 10 commits into from

Conversation

leotm
Copy link
Contributor

@leotm leotm commented Apr 10, 2023

Summary:

https://devblogs.microsoft.com/typescript/announcing-typescript-5-0

Changelog:

[GENERAL] [CHANGED] - Bump TypeScript in template from 4.8.4 to 5.0.4 and ESLint pkgs from 8.19.0 to 8.38.0

Test Plan:

Everything builds and runs as expected

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 10, 2023
@leotm leotm marked this pull request as ready for review April 10, 2023 20:17
@analysis-bot
Copy link

analysis-bot commented Apr 10, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,606,573 -16,428
android hermes armeabi-v7a 7,920,722 -15,638
android hermes x86 9,092,832 -16,815
android hermes x86_64 8,947,877 -16,797
android jsc arm64-v8a 9,173,833 -12,943
android jsc armeabi-v7a 8,365,493 -12,160
android jsc x86 9,231,488 -13,337
android jsc x86_64 9,490,181 -13,308

Base commit: 14b2b1c
Branch: main

Copy link
Contributor

@NickGerleman NickGerleman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also need to bump the version of the typescript eslint plug-in we bring in as well?

Previously, using TS 4.9 was enough to generate a warning with the resolved plug-in, but I don't recall how loose our constraints were, so maybe new apps will use a compatible version of the plug-in now.

Could we confirm linting a new project doesn't result in any warnings? Otherwise this LGTM.

@leotm
Copy link
Contributor Author

leotm commented Apr 11, 2023

great point ^ fixing the ESLint warning (to support TS 5) only needed bumping @typescript-eslint/parser to 5.55.0
(then other ESLint deps can be bumped alongside in fuller projects)

ref: https://github.com/typescript-eslint/typescript-eslint/releases/tag/v5.55.0

update TypeScript to 5.0 RC (typescript-eslint/typescript-eslint#6570) (36ef0e1)

@leotm
Copy link
Contributor Author

leotm commented Apr 11, 2023

can confirm no warnings in npx react-native@latest init RN0720RC1 --version 0.72.0-rc.1
after bumping to TS 5 https://github.com/leotm/RN0720RC1/commits/main

@leotm
Copy link
Contributor Author

leotm commented Apr 11, 2023

RN0720RC1 git:(main) npm list @typescript-eslint/parser
RN0720RC1@0.0.1 /Users/leo/Documents/GitHub/RN0720RC1
└─┬ @react-native/eslint-config@0.72.1
  ├─┬ @typescript-eslint/eslint-plugin@5.57.1
  │ └── @typescript-eslint/parser@5.57.1 deduped
  └── @typescript-eslint/parser@5.57.1

👌

@kelset
Copy link
Collaborator

kelset commented Apr 11, 2023

cc @lunaleaps

@kelset kelset requested a review from lunaleaps April 11, 2023 15:03
@leotm leotm requested a review from NickGerleman April 11, 2023 16:48
Co-authored-by: Luna <lunaleaps@gmail.com>
@kelset
Copy link
Collaborator

kelset commented Apr 12, 2023

Hey @leotm when you say

Everything builds and runs as expected

what do you mean? What is the "everything" here? I'm surprised that bumping to 5.x didn't need any types changes 🤔

@NickGerleman
Copy link
Contributor

➜ RN0720RC1 git:(main) npm list @typescript-eslint/parser
RN0720RC1@0.0.1 /Users/leo/Documents/GitHub/RN0720RC1
└─┬ @react-native/eslint-config@0.72.1
├─┬ @typescript-eslint/eslint-plugin@5.57.1
│ └── @typescript-eslint/parser@5.57.1 deduped
└── @typescript-eslint/parser@5.57.1



    
      
    

      
    

    
  
👌

This eslint config is also in the repo, so it can be bumped alongside this change.

@leotm
Copy link
Contributor Author

leotm commented Apr 12, 2023

my bad ^ was referring to

  • CI passing (this PR)
  • no TS compiler errors/warnings (demo'd in repro)
  • no ESLint errors/warnings (demo'd in repro)

and gave android/ios a test run in repro

@leotm
Copy link
Contributor Author

leotm commented Apr 12, 2023

been monitoring TS 5 nightly/dev/rc in https://github.com/leotm/react-native-template-new-architecture past few mo

which types changes we expecting?

edit: oops worth noting, template above on 0.71.6 atm (not 0.72.0-rc.1 like the repro)

@leotm
Copy link
Contributor Author

leotm commented Apr 12, 2023

bumped @react-native/eslint-config from 0.72.1 to 0.73.0 in repro, still LGTM
https://github.com/leotm/RN0720RC1/commits/main

we're already on

"@react-native/eslint-config": "^0.73.0",

done in

@leotm
Copy link
Contributor Author

leotm commented Apr 12, 2023

while we're at it bumped eslint from 8.19.0 to 8.38.0 in repro, still LGTM
https://github.com/leotm/RN0720RC1/commits/main

also done alongside in this pr, feel free to revert if needed

@leotm leotm requested a review from lunaleaps April 12, 2023 10:09
@kelset
Copy link
Collaborator

kelset commented Apr 12, 2023

these types of bumps needs to be done repo wide, we can't misalign the template from the rest of the codebase.

@leotm
Copy link
Contributor Author

leotm commented Apr 16, 2023

sry bk from node congress berlin ^ makes sense like before and should be small w/o pkg lockfiles now

i'll bump remaining relevant pkgs and pray CI passes

@github-actions
Copy link

github-actions bot commented Apr 16, 2023

Warnings
⚠️ 🔒 package.json - Changes were made to package.json. This will require a manual import by a Facebook employee.

Generated by 🚫 dangerJS against 89b0d69

@kelset
Copy link
Collaborator

kelset commented Apr 17, 2023

Headsup, for typescript, you are missing the root package.json - it's still at 4.x there.

@leotm
Copy link
Contributor Author

leotm commented Apr 24, 2023

ci/circleci: test_android_template-NewArch-Release-JSC
node ./scripts/template/initialize.js can't fetch pkgs -> can't make

ci/circleci: test_ios_template-NewArch-Release-WithoutFlipper-Hermes-StaticLibraries
bundle install && bundle exec pod install can't connect to github

ci/circleci: test_windows
yarn test FabricUIManager.getFabricUIManager()).measureLayout mistakenly called

:suspect:

@facebook-github-bot
Copy link
Contributor

@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit to facebook/metro that referenced this pull request May 10, 2023
Summary:
https://devblogs.microsoft.com/typescript/announcing-typescript-5-0

## Changelog:

[GENERAL] [CHANGED] - Bump TypeScript in template from 4.8.4 to 5.0.4 and ESLint pkgs from 8.19.0 to 8.38.0

X-link: facebook/react-native#36862

Reviewed By: cortinico

Differential Revision: D45720238

Pulled By: NickGerleman

fbshipit-source-id: d38b60110434760fdedc84ad941e0918bb986a40
@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label May 10, 2023
@facebook-github-bot
Copy link
Contributor

@NickGerleman merged this pull request in d2e446d.

@leotm leotm deleted the patch-6 branch May 18, 2023 13:18
blakef pushed a commit to blakef/template that referenced this pull request Feb 28, 2024
Summary:
https://devblogs.microsoft.com/typescript/announcing-typescript-5-0

## Changelog:

[GENERAL] [CHANGED] - Bump TypeScript in template from 4.8.4 to 5.0.4 and ESLint pkgs from 8.19.0 to 8.38.0

Pull Request resolved: facebook/react-native#36862

Test Plan: Everything builds and runs as expected

Reviewed By: cortinico

Differential Revision: D45720238

Pulled By: NickGerleman

fbshipit-source-id: d38b60110434760fdedc84ad941e0918bb986a40

Original: facebook/react-native@d2e446d
blakef pushed a commit to react-native-community/template that referenced this pull request Feb 29, 2024
Summary:
https://devblogs.microsoft.com/typescript/announcing-typescript-5-0

## Changelog:

[GENERAL] [CHANGED] - Bump TypeScript in template from 4.8.4 to 5.0.4 and ESLint pkgs from 8.19.0 to 8.38.0

Pull Request resolved: facebook/react-native#36862

Test Plan: Everything builds and runs as expected

Reviewed By: cortinico

Differential Revision: D45720238

Pulled By: NickGerleman

fbshipit-source-id: d38b60110434760fdedc84ad941e0918bb986a40

Original-Commit: facebook/react-native@d2e446d
blakef pushed a commit to react-native-community/template that referenced this pull request Feb 29, 2024
Summary:
https://devblogs.microsoft.com/typescript/announcing-typescript-5-0

## Changelog:

[GENERAL] [CHANGED] - Bump TypeScript in template from 4.8.4 to 5.0.4 and ESLint pkgs from 8.19.0 to 8.38.0

Pull Request resolved: facebook/react-native#36862

Test Plan: Everything builds and runs as expected

Reviewed By: cortinico

Differential Revision: D45720238

Pulled By: NickGerleman

fbshipit-source-id: d38b60110434760fdedc84ad941e0918bb986a40

Original-Commit: facebook/react-native@d2e446d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants