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

chore: update project dependencies #360

Conversation

1abhishekpandey
Copy link
Contributor

@1abhishekpandey 1abhishekpandey commented Jul 12, 2024

Description of the change

  • Updated react-native package from 0.73.2 to 0.74.1.
  • Multiple other packages maintained by nx have also been to the newest versions.
  • CI:
    - Updated the Cocoapod version to 1.5.2 in test.yml. This was needed to solve issues with the GitHub Action.
    - Removed npm run check:circular step from build-and-quality-checks.yml. I'll explain more details about this later.
    - Replace the Setup CocoaPods step with this action: https://github.com/NiftyStack/install-cocoapods-action and also bump the ruby version to 3.3.2.
  • Addressed snyk issues:
    - Removed ip package. This was also flagged by the npm security check.
    - Removed inflight package.
    - Update the ws to required version which doesn't contain any issues.
  • Updated the node version to 20.0.0 20.15.1 in the .nvmrc file.
  • Created new sample apps: Due to the update in the various packages maintained by nx the Android and iOS sample apps were not building properly. To resolve that, we had to create a new sample app.
    - For example, flipper libraries have been removed, which is being used by the sample apps.
    - Most of the changes in the sample app are due to creating new apps.
    A few of the tweaks done previously in the Podfile (sample iOS app) e.g., related to the flipper are no longer required. Those tweaks are either removed or commented out (this should be removed in future iterations).
  • Package.json:
    - Removed the check:circular script and the corresponding madge package. Actually, madge had a transitive dependency on the requirejs, which has been flagged by the npm security check. Therefore, we had to remove this for now.
    - Removed storybook from the devDependencies section, as it contained some flagged dependencies. This package was added earlier by nx and now it doesn't seem to be needed.
    - Added overrides for various packages: micromatch, @nx/devkit, svgo, glob, tempfile, rimraf, data-urls and jsdom. I'll explain more details about these changes later.
    - Removed stale overrides.
  • After nx updated the various packages to a newer version, we got a bunch of deprecated packages and also one package (i.e., requirejs) was flagged by the npm security check. We solved most of such issues except a few deprecated ones which are used as a transitive dependency and we don't have any way to replace them on our own. Such dependencies should be removed in future. So when we perform a similar upgrade again in the future this should be resolved.
  • After bumping to the node version 20.15.1, more packages were flagged by the npm security check, so we fixed all of them:
    - trim-newlines: https://www.npmjs.com/package/trim-newlines?activeTab=versions
    - semver-regex: GHSA-44c6-4v22-4mhx
    - lodash.template: GHSA-35jh-r3h4-6jhm (need to override the git-raw-commits)
    - http-cache-semantics: GHSA-rc47-6667-2j5j
    - dot-prop: GHSA-ff7x-qrg7-qggm

NOTE: This PR change will not require any release.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Related issues

Fix #1

Checklists

Development

  • Lint rules pass locally
  • The code changed/added as part of this pull request has been covered with tests
  • All tests related to the changed code pass in development

Code review

  • This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
  • "Ready for review" label attached to the PR and reviewers mentioned in a comment
  • Changes have been reviewed by at least one other engineer
  • Issue from task tracker has a link to this pull request

It is required by the different modules
…address various issues

This will remove the ip package and address snyk and npm security issues
…kage to address various issues"

This reverts commit 31ff650.
…address various issues

This will remove the ip package and address snyk and npm security issues
This is necessary to fix the build issue in Android and iOS
It is required to check the circular dependencies
…heck

Required to solve the requirejs related npm security issue.
@1abhishekpandey 1abhishekpandey self-assigned this Jul 12, 2024
…onventional-changelog-for-jira in package.json
@1abhishekpandey 1abhishekpandey marked this pull request as ready for review July 15, 2024 08:33
@1abhishekpandey 1abhishekpandey requested a review from a team as a code owner July 15, 2024 08:33
@1abhishekpandey 1abhishekpandey marked this pull request as draft July 15, 2024 11:43
Also update the ruby-version to 3.3.2
…it-raw-commit

This is required to fix the nx affected command whcih generates the changelog
@1abhishekpandey 1abhishekpandey marked this pull request as ready for review July 15, 2024 13:41
@1abhishekpandey
Copy link
Contributor Author

@1abhishekpandey , address this issue too, https://sonarcloud.io/project/issues?sinceLeakPeriod=true&issueStatuses=OPEN%2CCONFIRMED&pullRequest=360&id=rudderlabs_rudder-sdk-react-native&open=AZCpE5EFdxZRoYEvVfdr

NX added that. I can remove it, but it'll be added again in the next update. From the past couple of iterations, I'm manually removing this. But this time I allowed it. Do you still propose to remove it @saikumarrs?

@saikumarrs
Copy link
Member

@1abhishekpandey , address this issue too, https://sonarcloud.io/project/issues?sinceLeakPeriod=true&issueStatuses=OPEN%2CCONFIRMED&pullRequest=360&id=rudderlabs_rudder-sdk-react-native&open=AZCpE5EFdxZRoYEvVfdr

NX added that. I can remove it, but it'll be added again in the next update. From the past couple of iterations, I'm manually removing this. But this time I allowed it. Do you still propose to remove it @saikumarrs?

Are you sure it is Nx that added it? It is not supposed to make any changes to the code directly. Maybe it is something else.

@1abhishekpandey
Copy link
Contributor Author

@1abhishekpandey , address this issue too, https://sonarcloud.io/project/issues?sinceLeakPeriod=true&issueStatuses=OPEN%2CCONFIRMED&pullRequest=360&id=rudderlabs_rudder-sdk-react-native&open=AZCpE5EFdxZRoYEvVfdr

NX added that. I can remove it, but it'll be added again in the next update. From the past couple of iterations, I'm manually removing this. But this time I allowed it. Do you still propose to remove it @saikumarrs?

Are you sure it is Nx that added it? It is not supposed to make any changes to the code directly. Maybe it is something else.

Yes, I ran this nx command to create a new sample app (I've mentioned the reason in the PR description) nx g @nx/react-native:app --directory="apps" example. More details can be found in the nx doc: https://nx.dev/nx-api/react-native#generating-applications. Please let me know if you still suggest removing this.

@saikumarrs
Copy link
Member

@1abhishekpandey , address this issue too, https://sonarcloud.io/project/issues?sinceLeakPeriod=true&issueStatuses=OPEN%2CCONFIRMED&pullRequest=360&id=rudderlabs_rudder-sdk-react-native&open=AZCpE5EFdxZRoYEvVfdr

NX added that. I can remove it, but it'll be added again in the next update. From the past couple of iterations, I'm manually removing this. But this time I allowed it. Do you still propose to remove it @saikumarrs?

Are you sure it is Nx that added it? It is not supposed to make any changes to the code directly. Maybe it is something else.

Yes, I ran this nx command to create a new sample app (I've mentioned the reason in the PR description) nx g @nx/react-native:app --directory="apps" example. More details can be found in the nx doc: https://nx.dev/nx-api/react-native#generating-applications. Please let me know if you still suggest removing this.

Yeah, please remove it as it is against our lint rules.

Copy link

@1abhishekpandey 1abhishekpandey merged commit dfdf3dc into develop Jul 18, 2024
9 checks passed
@1abhishekpandey 1abhishekpandey deleted the feat/sdk-2077-update-react-native-package-to-the-latest-version branch July 18, 2024 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants