fix: use node resolver to prevent escaped traversal errors#506
Conversation
📝 WalkthroughWalkthroughUpdated a lint suppression comment in the config builder and refactored the computation of the internal base path in Metro resolvers by replacing context-dependent resolver calls with Node's Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/uniwind/src/metro/resolvers.ts (1)
47-53: Consider extracting the duplicated base-path resolution.The identical
try/catchblock is duplicated in bothnativeResolver(lines 47-53) andwebResolver(lines 95-101). A small helper would keep both paths in sync if the strategy evolves again.♻️ Proposed refactor
let cachedInternalBasePath: string | null = null + +const getInternalBasePath = () => { + if (cachedInternalBasePath === null) { + try { + cachedInternalBasePath = dirname(require.resolve('uniwind/package.json')) + } catch { + cachedInternalBasePath = '' + } + } + return cachedInternalBasePath +}Then replace both inline blocks with
const basePath = getInternalBasePath()and usebasePathin the subsequent checks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/uniwind/src/metro/resolvers.ts` around lines 47 - 53, Extract the duplicated try/catch resolution into a small helper (e.g., getInternalBasePath) that returns the dirname of require.resolve('uniwind/package.json') or '' on failure and uses a module-scoped cache similar to cachedInternalBasePath; then replace the inline blocks inside nativeResolver and webResolver with const basePath = getInternalBasePath() (or use the cached value returned) and update subsequent checks to reference basePath so both resolvers share the same resolution logic.packages/uniwind/src/core/config/config.common.ts (1)
104-107: Lint suppression correction is appropriate; underscore prefix would align with existing conventions.The change to
typescript/no-unused-varscorrectly addresses the unused parameter. However, this file already uses underscore prefixes for intentionally unused parameters (line 109:_: GenerateStyleSheetsCallback), whileupdateInsetsandupdateCSSVariablesuse lint suppression comments. For consistency:Optional refactor for consistency
- // oxlint-disable-next-line typescript/no-unused-vars - updateInsets(insets: Insets) { + updateInsets(_insets: Insets) { // noop }This matches the pattern at line 109 and eliminates the suppression comment entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/uniwind/src/core/config/config.common.ts` around lines 104 - 107, Replace the inline lint suppression for unused parameters with the underscore-prefixed parameter naming used elsewhere: change the signatures of updateInsets(insets: Insets) and updateCSSVariables(cssVars: CSSVariables) to use _insets and _cssVars respectively (matching the existing pattern used for _: GenerateStyleSheetsCallback) so the compiler understands the parameters are intentionally unused and the types remain declared without needing the typescript/no-unused-vars disable comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/uniwind/src/core/config/config.common.ts`:
- Around line 104-107: Replace the inline lint suppression for unused parameters
with the underscore-prefixed parameter naming used elsewhere: change the
signatures of updateInsets(insets: Insets) and updateCSSVariables(cssVars:
CSSVariables) to use _insets and _cssVars respectively (matching the existing
pattern used for _: GenerateStyleSheetsCallback) so the compiler understands the
parameters are intentionally unused and the types remain declared without
needing the typescript/no-unused-vars disable comment.
In `@packages/uniwind/src/metro/resolvers.ts`:
- Around line 47-53: Extract the duplicated try/catch resolution into a small
helper (e.g., getInternalBasePath) that returns the dirname of
require.resolve('uniwind/package.json') or '' on failure and uses a
module-scoped cache similar to cachedInternalBasePath; then replace the inline
blocks inside nativeResolver and webResolver with const basePath =
getInternalBasePath() (or use the cached value returned) and update subsequent
checks to reference basePath so both resolvers share the same resolution logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 822e3733-db72-4ec8-9494-0126e361004a
📒 Files selected for processing (2)
packages/uniwind/src/core/config/config.common.tspackages/uniwind/src/metro/resolvers.ts
|
🚀 This pull request is included in v1.6.3. See Release v1.6.3 for release notes. |
fixes #505
Summary by CodeRabbit