-
Notifications
You must be signed in to change notification settings - Fork 148
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
fix: npmrc registry url #2112
fix: npmrc registry url #2112
Conversation
|
✅ PR title follows Conventional Commits specification. |
packages/blade/package.json
Outdated
"react": "18.2.0", | ||
"@types/react": "17.0.38", |
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.
We should target upgrading types to v18
@@ -0,0 +1 @@ | |||
@razorpay:registry=https://registry.npmjs.org/ |
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.
Isn't the default npmjs registry?
@@ -250,7 +250,7 @@ | |||
"react-native": "0.72.3", | |||
"react-native-gesture-handler": "2.9.0", | |||
"react-native-reanimated": "3.4.2", | |||
"react-native-svg": "12.3.0", | |||
"react-native-svg": "15.1.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.
Why?
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.
v12 types are not compatible with react18 types.
plus we and most of our consumers are using react-native >v70, thus according to this table we should be using v15 of rnsvg.
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 f72c1b5:
|
Bundle Size ReportUpdated Components
|
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.
Looks good overall. Can you check if we can do something about those any
s 🙈
@@ -2,7 +2,7 @@ name: Blade PR Title Check | |||
|
|||
on: | |||
pull_request_target: | |||
types: [open, edited, synchronize] | |||
types: [opened, edited, synchronize] |
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 was incorrect all along? how was it working 😄 ?
"@types/react-native": "0.64.4", | ||
"@types/styled-components": "5.1.25", | ||
"@types/styled-components-react-native": "5.1.3", | ||
"@types/react": "18.2.24", |
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.
styled-components, react-native types not needed?
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.
Oh just saw they are added in blade's package.json. So why is types/react needed here but types/styled-components are in blade's package.json?
Description
Changes
Additional Information
Component Checklist