-
-
Notifications
You must be signed in to change notification settings - Fork 355
Lint rules and code style for expo sample app #5484
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
base: main
Are you sure you want to change the base?
Conversation
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Features
- Lint rules and code style for expo sample app ([#5484](https://github.com/getsentry/sentry-react-native/pull/5484))If none of the above apply, you can opt out of this check by adding |
|
@sentry review |
samples/expo/package.json
Outdated
| "@react-native/eslint-config": "0.79.1", | ||
| "@sentry/babel-plugin-component-annotate": "4.6.1", | ||
| "@types/node": "20.10.4", | ||
| "@typescript-eslint/eslint-plugin": "^7.18.0", |
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.
I'm getting the following error when running lint locally (I think it's the same on CI):
~/git/sentry-react-native/samples/expo$ yarn lint
=============
WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.
You may find that it works just fine, or you may not.
SUPPORTED TYPESCRIPT VERSIONS: >=4.7.4 <5.6.0
YOUR TYPESCRIPT VERSION: 5.8.3
Please only submit bug reports when using the officially supported version.
=============
I think we might need to also bump the parser to align with the typescript version. What has worked locally for me is the following
diff --git a/samples/expo/.eslintrc.js b/samples/expo/.eslintrc.js
index 99a35e6e..be4b8ca4 100644
--- a/samples/expo/.eslintrc.js
+++ b/samples/expo/.eslintrc.js
@@ -11,6 +11,9 @@ module.exports = {
'no-shadow': 'off',
'no-undef': 'off',
quotes: [2, 'single', { avoidEscape: true }],
+ // Disable deprecated rules removed in @typescript-eslint v8
+ '@typescript-eslint/func-call-spacing': 'off',
+ '@typescript-eslint/ban-types': 'off',
},
},
],
diff --git a/samples/expo/package.json b/samples/expo/package.json
index f0ae4168..aa116d90 100644
--- a/samples/expo/package.json
+++ b/samples/expo/package.json
@@ -48,10 +48,13 @@
"@react-native/eslint-config": "0.79.1",
"@sentry/babel-plugin-component-annotate": "4.6.1",
"@types/node": "20.10.4",
- "@typescript-eslint/eslint-plugin": "^7.18.0",
- "@typescript-eslint/parser": "^7.18.0",
+ "@typescript-eslint/eslint-plugin": "^8.50.0",
+ "@typescript-eslint/parser": "^8.50.0",
"eslint": "^8.57.0",
"eslint-config-expo": "^7.1.2",
+ "eslint-import-resolver-typescript": "^4.4.4",
+ "eslint-plugin-import": "^2.32.0",
+ "eslint-plugin-jest": "^29.5.0",
"prettier": "2.8.8",
"sentry-react-native-samples-utils": "workspace:^"
},
| import * as Sentry from '@sentry/react-native'; | ||
| import { SeverityLevel } from '@sentry/core'; | ||
| import { SeverityLevel } from '@sentry/react-native'; | ||
|
|
||
| export const setScopeProperties = () => { | ||
| const dateString = new Date().toString(); |
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.
Bug: The import of SeverityLevel from @sentry/react-native will fail because the type is not exported from that package, causing a crash.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The file setScopeProperties.ts attempts to import SeverityLevel from the @sentry/react-native package. However, SeverityLevel is not actually exported from this package. This will cause a module resolution error when the application attempts to load this file, leading to an immediate crash on startup. The type is used in several places within the file for type assertions, such as 'info' as SeverityLevel.
💡 Suggested Fix
Revert the import statement for SeverityLevel to its original source, @sentry/core, which correctly exports the type. Change import { SeverityLevel } from '@sentry/react-native'; back to import { SeverityLevel } from '@sentry/core';.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: samples/expo/utils/setScopeProperties.ts#L1-L5
Potential issue: The file `setScopeProperties.ts` attempts to import `SeverityLevel`
from the `@sentry/react-native` package. However, `SeverityLevel` is not actually
exported from this package. This will cause a module resolution error when the
application attempts to load this file, leading to an immediate crash on startup. The
type is used in several places within the file for type assertions, such as `'info' as
SeverityLevel`.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7817854
📢 Type of change
📜 Description
Fixes #5436
Apparently only the expo app needed this update because lint rules were already specified for
samples/react-nativeandsamples/react-native-macos📝 Checklist
sendDefaultPIIis enabled🔮 Next steps