fix: Move unused dependencies to devDependencies #171
Conversation
WalkthroughFour runtime dependencies—hermes-engine, react-native-safe-area-context, react-native-screens, and react-native-webview—were moved from the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
PR Summary: Summary: Changes:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| "babel-jest": "^29.5.0", | ||
| "eslint": "^8.44.0", | ||
| "eslint-plugin-simple-import-sort": "^10.0.0", | ||
| "hermes-engine": "^0.11.0", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify these dependencies are not used by the library's source code and confirm the ideal location.
Moving these four packages to devDependencies ensures they won't be installed as runtime dependencies for library consumers. However, per the PR objectives, these should ideally be in the example app's package.json rather than the root devDependencies, since they appear to be example/sample-app-only dependencies.
Verify the following:
- Confirm that the library's source code in
src/does not import any of these packages: hermes-engine, react-native-safe-area-context, react-native-screens, react-native-webview - Check whether an example app directory exists with its own
package.jsonthat should contain these dependencies - Confirm that moving these to
devDependencies(rather than the example app'spackage.json) is the intended final state, or if this is a temporary measure
Also applies to: 68-70
🏁 Script executed:
#!/bin/bash
# Description: Verify that the four moved packages are not imported in library source code
# Search for imports of the four moved packages in the src/ directory
echo "=== Searching for hermes-engine imports ==="
rg -n --type=ts --type=tsx --type=js --type=jsx 'from.*hermes|import.*hermes' src/
echo "=== Searching for react-native-safe-area-context imports ==="
rg -n --type=ts --type=tsx --type=js --type=jsx 'from.*safe-area-context|import.*safe-area-context' src/
echo "=== Searching for react-native-screens imports ==="
rg -n --type=ts --type=tsx --type=js --type=jsx 'from.*react-native-screens|import.*react-native-screens' src/
echo "=== Searching for react-native-webview imports ==="
rg -n --type=ts --type=tsx --type=js --type=jsx 'from.*react-native-webview|import.*react-native-webview' src/
echo "=== Checking if example directory exists ==="
fd -t d '^example$' .
echo "=== Listing contents of example directory if present ==="
find . -maxdepth 2 -type d -name example -exec ls -la {} \;Length of output: 2088
🏁 Script executed:
#!/bin/bash
# Description: Verify package locations and verify no imports in library source
# Fix: Use simpler approach without problematic type flags
echo "=== Checking root package.json devDependencies section ==="
grep -A 30 '"devDependencies"' package.json | grep -E 'hermes-engine|react-native-safe-area-context|react-native-screens|react-native-webview'
echo "=== Checking example/package.json for these packages ==="
grep -E 'hermes-engine|react-native-safe-area-context|react-native-screens|react-native-webview' example/package.json
echo "=== Searching for package imports in src/ (hermes) ==="
rg -n 'hermes' src/ 2>/dev/null || echo "No hermes matches"
echo "=== Searching for package imports in src/ (safe-area-context) ==="
rg -n 'safe-area-context' src/ 2>/dev/null || echo "No safe-area-context matches"
echo "=== Searching for package imports in src/ (screens) ==="
rg -n 'react-native-screens' src/ 2>/dev/null || echo "No react-native-screens matches"
echo "=== Searching for package imports in src/ (webview) ==="
rg -n 'react-native-webview' src/ 2>/dev/null || echo "No react-native-webview matches"
echo "=== Full example/package.json contents ==="
cat example/package.jsonLength of output: 3016
Move these packages to example/package.json instead of root devDependencies.
Verification confirms the library source code does not import any of these packages, so moving them from dependencies was correct. However, moving them to root devDependencies is problematic:
- hermes-engine is now duplicated: exists in both root
devDependencies(from this PR) andexample/package.json(existing) - react-native-safe-area-context, react-native-screens, react-native-webview are in root
devDependenciesbut missing fromexample/package.jsonwhere the example app needs them - Root
devDependenciesshould only contain dev tooling (linters, compilers, etc.), not app dependencies
Add these packages to example/package.json where the example app requires them, remove the duplication for hermes-engine, and remove all three from root devDependencies.
🤖 Prompt for AI Agents
In package.json around line 61, the PR incorrectly added app libraries to the
root devDependencies; remove "hermes-engine", "react-native-safe-area-context",
"react-native-screens", and "react-native-webview" from the root
devDependencies, ensure "hermes-engine" is not duplicated (keep it only in
example/package.json), and add "react-native-safe-area-context",
"react-native-screens", and "react-native-webview" into example/package.json (as
dependencies or devDependencies as appropriate for the example app) so the
example app has the required packages and the root devDependencies only contain
tooling.
|
Reviewed up to commit:70ad21480fa5548321500e32a6304a2f0390780b Additional Suggestionscripts/check-requirements.sh, line:285-317The script checks for the string 'minSdkVersion 23' in the react-native-screens build.gradle to confirm patch application, but the patches now update the minSdkVersion to 24. Update the grep condition to search for 'minSdkVersion 24' to ensure correct detection.if grep -q "minSdkVersion 24" "$PROJECT_ROOT/sample/node_modules/react-native-screens/android/build.gradle"; then Few more points:
|
peterlazar1993
changed the title
1 participant
Howdy folks, thank you for the wonderful library and congratulations on enabling Fabric compaitablity.
I want to bring to your attention an issue I noticed in how the library is packaged.
Recent releases of the package specifies some dependencies, that are not used by the library. They are only used by the sample/example apps.
They should be specified in the sample app's package.json instead, or atleast moved to devDependencies.
On top of this, two dependencies 'npm' and 'install' are also installed. This is most definitely a mistake. At some point
yarn add npm installwas ran and these packages were added.Please consider using npm/yarn workspaces.
Summary by CodeRabbit