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

Disable ESLint no-undef for TypeScript files #32655

Conversation

fiznool
Copy link
Contributor

@fiznool fiznool commented Nov 24, 2021

Summary

The typescript-eslint project recommends that no-undef is disabled for TypeScript files, since TypeScript itself will perform this check. Disabling this config property has two benefits:

  • Undefined variables and types will only be reported once, by the TypeScript compiler. Currently they are reported twice: once by TypeScript, and once by ESLint:

Screenshot 2021-11-24 at 12 38 22

  • Types that are declared globally by React Native will no longer be erroneously reported as undefined - this is currently the case for some types, e.g. Blob:

Screenshot 2021-11-24 at 12 40 04

Changelog

[General] [Fixed] - ESLint no-undef rule clashing with TypeScript compiler for TS files

Test Plan

  • Before: ESLint reporting on undefined variables in TypeScript files
  • After: ESLint no longer reporting on undefined variables in TypeScript files

The `typescript-eslint` project recommends that `no-undef` is disabled for TypeScript files, since TypeScript itself will perform this check.
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Nov 24, 2021
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 749a920
Branch: main

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,527,085 +191
android hermes armeabi-v7a 7,830,820 -418
android hermes x86 8,997,004 -134
android hermes x86_64 8,942,914 -225
android jsc arm64-v8a 9,841,455 +559
android jsc armeabi-v7a 8,806,400 -319
android jsc x86 9,791,318 -121
android jsc x86_64 10,393,967 -227

Base commit: 749a920
Branch: main

@facebook-github-bot
Copy link
Contributor

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

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @fiznool in ae67c5a.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Dec 2, 2021
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Nov 2, 2022
Summary:
This change moves the default new application template in OSS from Flow to TypeScript. This better aligns with the communities usage, and aligns to the great work that has been happening for TS codegen and built-in types. This is largely derived from [`react-native-community/react-native-template-typescript`](https://github.com/react-native-community/react-native-template-typescript), maintained by radko93, with a couple of changes.
1. Updated `types/*` devDependencies to match bumped libraries (e.g. Jest 26 to 20).
2. Removes `types/react-native`
3. Removed explicit `moduleFileExtensions` to Jest config in package.json (TS and TSX and added by default in current versions)
4. Removes overrides to eslint config to disable `no-shadow` and `no-undef`, since this was fixed in the underlying eslint config with facebook#32644 and facebook#32655
5. Re-translated `App.js` to be a direct translation (e.g. still a raw function instead of React.FC which is sometimes discouraged)
6. Aligns completely to `tsconfig/react-native` maintained configuration (We no longer have the opinionated override of `skipLibCheck`). The important settings are that `strict` is enabled for, but `allowJS` is also enabled to let users import JS modules without anything unexpected. `esModuleInterop` is enabled, which is needed for consistency with Babel import transformations used by Metro. Consistent with our [current documentation](https://reactnative.dev/docs/typescript) built against the community template, `tsc` will typecheck your code without emitting(building) anything.

I discovered through this change that the way 'Utilities' is imported in typings doesn't seem to work in the template app use-case (but works in the RN repo?). Fixed up those imports.

We also partially revert facebook@f49b251 which deleted typings. Checking to make sure we keep these in the appropriate place in the future.

[Documentation](https://reactnative.dev/docs/typescript) will need to be updated as a followup.

Changelog:
[General][Changed] - Use Vanilla JS in the App Template

Pull Request resolved: facebook#35165

Test Plan: Added usage of `tsc` and `jest` when validating a newly built app. Passes in CircleCI `test_js` job. This also meant removing some cases of copying packages into the users app to pull in the root Metro config (we seem to be able to use the template Metro config consistent with what real apps would get).

Differential Revision: D40911951

Pulled By: NickGerleman

fbshipit-source-id: a989523e99ad0045ab4d345aeb2481ef3ca958ca
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Nov 2, 2022
Summary:
This change moves the default new application template in OSS from Flow to TypeScript. This better aligns with the communities usage, and aligns to the great work that has been happening for TS codegen and built-in types. This is largely derived from [`react-native-community/react-native-template-typescript`](https://github.com/react-native-community/react-native-template-typescript), maintained by radko93, with a couple of changes.
1. Updated `types/*` devDependencies to match bumped libraries (e.g. Jest 26 to 20).
2. Removes `types/react-native`
3. Removed explicit `moduleFileExtensions` to Jest config in package.json (TS and TSX and added by default in current versions)
4. Removes overrides to eslint config to disable `no-shadow` and `no-undef`, since this was fixed in the underlying eslint config with facebook#32644 and facebook#32655
5. Re-translated `App.js` to be a direct translation (e.g. still a raw function instead of React.FC which is sometimes discouraged)
6. Aligns completely to `tsconfig/react-native` maintained configuration (We no longer have the opinionated override of `skipLibCheck`). The important settings are that `strict` is enabled for, but `allowJS` is also enabled to let users import JS modules without anything unexpected. `esModuleInterop` is enabled, which is needed for consistency with Babel import transformations used by Metro. Consistent with our [current documentation](https://reactnative.dev/docs/typescript) built against the community template, `tsc` will typecheck your code without emitting(building) anything.

I discovered through this change that the way 'Utilities' is imported in typings doesn't seem to work in the template app use-case (but works in the RN repo?). Fixed up those imports.

We also partially revert facebook@f49b251 which deleted typings. Checking to make sure we keep these in the appropriate place in the future.

[Documentation](https://reactnative.dev/docs/typescript) will need to be updated as a followup.

Changelog:
[General][Changed] - Use Vanilla JS in the App Template

Pull Request resolved: facebook#35165

Test Plan: Added usage of `tsc` and `jest` when validating a newly built app. Passes in CircleCI `test_js` job. This also meant removing some cases of copying packages into the users app to pull in the root Metro config (we seem to be able to use the template Metro config consistent with what real apps would get).

Differential Revision: D40911951

Pulled By: NickGerleman

fbshipit-source-id: 81136e2aa2602d074107e1fbe8110498406e6ac9
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Nov 2, 2022
Summary:
This change moves the default new application template in OSS from Flow to TypeScript. This better aligns with the communities usage, and aligns to the great work that has been happening for TS codegen and built-in types. This is largely derived from [`react-native-community/react-native-template-typescript`](https://github.com/react-native-community/react-native-template-typescript), maintained by radko93, with a couple of changes.
1. Updated `types/*` devDependencies to match bumped libraries (e.g. Jest 26 to 20).
2. Removes `types/react-native`
3. Removed explicit `moduleFileExtensions` to Jest config in package.json (TS and TSX and added by default in current versions)
4. Removes overrides to eslint config to disable `no-shadow` and `no-undef`, since this was fixed in the underlying eslint config with facebook#32644 and facebook#32655
5. Re-translated `App.js` to be a direct translation (e.g. still a raw function instead of React.FC which is sometimes discouraged)
6. Aligns completely to `tsconfig/react-native` maintained configuration (We no longer have the opinionated override of `skipLibCheck`). The important settings are that `strict` is enabled for, but `allowJS` is also enabled to let users import JS modules without anything unexpected. `esModuleInterop` is enabled, which is needed for consistency with Babel import transformations used by Metro. Consistent with our [current documentation](https://reactnative.dev/docs/typescript) built against the community template, `tsc` will typecheck your code without emitting(building) anything.

I discovered through this change that the way 'Utilities' is imported in typings doesn't seem to work in the template app use-case (but works in the RN repo?). Fixed up those imports.

We also partially revert facebook@f49b251 which deleted typings. Checking to make sure we keep these in the appropriate place in the future.

[Documentation](https://reactnative.dev/docs/typescript) will need to be updated as a followup.

Changelog:
[General][Changed] - Use Vanilla JS in the App Template

Pull Request resolved: facebook#35165

Test Plan: Added usage of `tsc` and `jest` when validating a newly built app. Passes in CircleCI `test_js` job. This also meant removing some cases of copying packages into the users app to pull in the root Metro config (we seem to be able to use the template Metro config consistent with what real apps would get).

Differential Revision: D40911951

Pulled By: NickGerleman

fbshipit-source-id: 05bed911249583b02d2c180a27f955e2e36c0cc4
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Nov 2, 2022
Summary:
This change moves the default new application template in OSS from Flow to TypeScript. This better aligns with the communities usage, and aligns to the great work that has been happening for TS codegen and built-in types. This used [`react-native-community/react-native-template-typescript`](https://github.com/react-native-community/react-native-template-typescript) as a main reference, maintained by radko93. A few things are different:
1. Updated `types/*` devDependencies to match bumped libraries (e.g. Jest 26 to 20).
2. Removed `types/react-native`
3. Removed explicit `moduleFileExtensions` to Jest config in package.json (TS and TSX and added by default in current versions)
4. Removed overrides to eslint config to disable `no-shadow` and `no-undef`, since this was fixed in the underlying eslint config with facebook#32644 and facebook#32655
5. Re-translated `App.js` to be a direct translation (e.g. still a raw function instead of React.FC which is sometimes discouraged)
6. Aligns completely to `tsconfig/react-native` maintained configuration (We no longer have the opinionated override of `skipLibCheck`). The important settings are that `strict` is enabled for, but `allowJS` is also enabled to let users import JS modules without anything unexpected. `esModuleInterop` is enabled, which is needed for consistency with Babel import transformations used by Metro. Consistent with our [current documentation](https://reactnative.dev/docs/typescript) built against the community template, `tsc` will typecheck your code without emitting(building) anything.

[Documentation](https://reactnative.dev/docs/typescript) will need to be updated as a followup.

Changelog:
[General][Changed] - Use TypeScript by default for new applications

Pull Request resolved: facebook#35165

Test Plan: Added usage of `tsc` and `jest` when validating a newly built app. Passes in CircleCI `test_js` job. This also meant removing some cases of copying packages into the users app to pull in the root Metro config (we seem to be able to use the template Metro config consistent with what real apps would get).

Reviewed By: cortinico

Differential Revision: D40911951

Pulled By: NickGerleman

fbshipit-source-id: 08d4624bc08163c7dc1272f605bc3a8e9a89173b
facebook-github-bot pushed a commit that referenced this pull request Nov 3, 2022
Summary:
This change moves the default new application template in OSS from Flow to TypeScript. This better aligns with the communities usage, and aligns to the great work that has been happening for TS codegen and built-in types. This used [`react-native-community/react-native-template-typescript`](https://github.com/react-native-community/react-native-template-typescript) as a main reference, maintained by radko93. A few things are different:
1. Updated `types/*` devDependencies to match bumped libraries (e.g. Jest 26 to 20).
2. Removed `types/react-native`
3. Removed explicit `moduleFileExtensions` to Jest config in package.json (TS and TSX and added by default in current versions)
4. Removed overrides to eslint config to disable `no-shadow` and `no-undef`, since this was fixed in the underlying eslint config with #32644 and #32655
5. Re-translated `App.js` to be a direct translation (e.g. still a raw function instead of React.FC which is sometimes discouraged)
6. Aligns completely to `tsconfig/react-native` maintained configuration (We no longer have the opinionated override of `skipLibCheck`). The important settings are that `strict` is enabled for, but `allowJS` is also enabled to let users import JS modules without anything unexpected. `esModuleInterop` is enabled, which is needed for consistency with Babel import transformations used by Metro. Consistent with our [current documentation](https://reactnative.dev/docs/typescript) built against the community template, `tsc` will typecheck your code without emitting(building) anything.

[Documentation](https://reactnative.dev/docs/typescript) will need to be updated as a followup.

Changelog:
[General][Changed] - Use TypeScript by default for new applications

Pull Request resolved: #35165

Test Plan: Added usage of `tsc` and `jest` when validating a newly built app. Passes in CircleCI `test_js` job. This also meant removing some cases of copying packages into the users app to pull in the root Metro config (we seem to be able to use the template Metro config consistent with what real apps would get).

Reviewed By: cortinico

Differential Revision: D40911951

Pulled By: NickGerleman

fbshipit-source-id: 15994534235695e91cf994ad06ba2183dfc89a50
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
This change moves the default new application template in OSS from Flow to TypeScript. This better aligns with the communities usage, and aligns to the great work that has been happening for TS codegen and built-in types. This used [`react-native-community/react-native-template-typescript`](https://github.com/react-native-community/react-native-template-typescript) as a main reference, maintained by radko93. A few things are different:
1. Updated `types/*` devDependencies to match bumped libraries (e.g. Jest 26 to 20).
2. Removed `types/react-native`
3. Removed explicit `moduleFileExtensions` to Jest config in package.json (TS and TSX and added by default in current versions)
4. Removed overrides to eslint config to disable `no-shadow` and `no-undef`, since this was fixed in the underlying eslint config with facebook#32644 and facebook#32655
5. Re-translated `App.js` to be a direct translation (e.g. still a raw function instead of React.FC which is sometimes discouraged)
6. Aligns completely to `tsconfig/react-native` maintained configuration (We no longer have the opinionated override of `skipLibCheck`). The important settings are that `strict` is enabled for, but `allowJS` is also enabled to let users import JS modules without anything unexpected. `esModuleInterop` is enabled, which is needed for consistency with Babel import transformations used by Metro. Consistent with our [current documentation](https://reactnative.dev/docs/typescript) built against the community template, `tsc` will typecheck your code without emitting(building) anything.

[Documentation](https://reactnative.dev/docs/typescript) will need to be updated as a followup.

Changelog:
[General][Changed] - Use TypeScript by default for new applications

Pull Request resolved: facebook#35165

Test Plan: Added usage of `tsc` and `jest` when validating a newly built app. Passes in CircleCI `test_js` job. This also meant removing some cases of copying packages into the users app to pull in the root Metro config (we seem to be able to use the template Metro config consistent with what real apps would get).

Reviewed By: cortinico

Differential Revision: D40911951

Pulled By: NickGerleman

fbshipit-source-id: 15994534235695e91cf994ad06ba2183dfc89a50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants