feat: add support for variables in media queries#335
Conversation
📝 WalkthroughWalkthroughThis PR adds support for CSS variables inside media queries in UniWind. It introduces recursive media query condition processing in the compiler and defers variable resolution for media-query-backed declarations, resolving them after all style rules are processed. Comprehensive test coverage validates responsive typography and styling across multiple media query scenarios. Changes
Sequence DiagramsequenceDiagram
participant Compiler as Style Compiler
participant ProcBuilder as ProcessorBuilder
participant MqProc as MediaQuery Processor
participant VarStore as Variable Storage
participant Finalizer as Style Finalizer
Compiler->>ProcBuilder: process stylesheet
ProcBuilder->>ProcBuilder: iterate style rules
ProcBuilder->>MqProc: processCondition(condition)
MqProc->>MqProc: check condition.type
alt operation type
MqProc->>MqProc: recursively process nested
else feature type
MqProc->>ProcBuilder: process range/plain values
end
ProcBuilder->>ProcBuilder: hasMediaQuery check
alt media query exists
ProcBuilder->>VarStore: store in varsWithMediaQueries
ProcBuilder->>ProcBuilder: track in pendingVarReferences
else no media query
ProcBuilder->>ProcBuilder: assign style directly
end
ProcBuilder->>ProcBuilder: finalize all rules
ProcBuilder->>Finalizer: iterate pendingVarReferences
Finalizer->>VarStore: retrieve varsWithMediaQueries
Finalizer->>ProcBuilder: splice into class styles
Finalizer->>ProcBuilder: clear pending references
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
divineniiquaye
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/uniwind/src/metro/processor/processor.ts (2)
83-149: Track all CSS var references (arrays/objects + multiple matches).
Right now only the firstthis[...]in a top‑level string is detected. Vars embedded in arrays/objects (e.g., transforms, shadows) or multiple vars in one value won’t be tracked, so media‑query var updates won’t be injected for those classes.🐛 Proposed fix
- const match = typeof value === 'string' ? value.match(/this\[`(.*?)`\]/) : null - - if (match && !isVar) { + const varRefs = !isVar ? this.extractVarRefs(value) : [] + if (varRefs.length > 0) { const className = this.declarationConfig.className if (className === null) { return } @@ - const classVars = this.pendingVarReferences.get(className)! - const varName = match[1]! - - if (!classVars.includes(varName)) { - classVars.push(varName) - } + const classVars = this.pendingVarReferences.get(className)! + for (const varName of varRefs) { + if (!classVars.includes(varName)) { + classVars.push(varName) + } + } }Add a helper (near
parseDeclaration):private extractVarRefs(value: unknown): Array<string> { const refs = new Set<string>() const visit = (v: any) => { if (typeof v === 'string') { for (const m of v.matchAll(/this\[(?:`|"|')([^`"']+)(?:`|"|')\]/g)) { if (m[1]?.startsWith('--')) refs.add(m[1]) } return } if (Array.isArray(v)) return v.forEach(visit) if (v && typeof v === 'object') Object.values(v).forEach(visit) } visit(value) return [...refs] }
274-279: Preserve outer declaration context for nested@media.
Resetting togetDeclarationConfig()here drops any outer media context when@mediais nested (including CSS nesting). That can cause subsequent rules in the outer block to lose their media constraints.🐛 Proposed fix
- this.declarationConfig.mediaQueries.push(...mediaQueries) - rule.value.rules.forEach(rule => this.parseRuleRec(rule)) - this.declarationConfig = this.getDeclarationConfig() + const prevConfig = this.declarationConfig + this.declarationConfig = { + ...prevConfig, + mediaQueries: [...prevConfig.mediaQueries, ...mediaQueries], + } + rule.value.rules.forEach(rule => this.parseRuleRec(rule)) + this.declarationConfig = prevConfigpackages/uniwind/src/metro/processor/mq.ts (1)
1-40: Handleorandnotoperators in media conditions to apply styles correctly.
processConditiondoes not check theoperatorfield in'operation'type conditions, treatingorasand(all conditions applied to the same resolver). It also ignores the'not'condition type entirely. This causesoroperations to incorrectly apply all branches simultaneously andnotconditions to be silently skipped. For example,(max-width: 600px or min-width: 1200px)would incorrectly set both constraints.Either split
oroperations into multiple resolvers, or detect and explicitly handle/reject unsupported operators.
🤖 Fix all issues with AI agents
In `@packages/uniwind/tests/media-queries/color-scheme.test.tsx`:
- Around line 26-29: Replace the eval(...) call that invokes Uniwind.__reinit
with a new Function(...) pattern and add the same linting disable/enable
comments used in production tests so the test passes linting; specifically
change the eval invocation that builds and executes the runtime string for
Uniwind.__reinit(rt => ${virtualCode}, ['light','dark']) to use new
Function(...) to return and call the function, and wrap that call with the same
eslint-disable/enable comments used elsewhere in tests; apply the identical
change to the other test files noted (variables, orientation, min-width,
max-width, combined, setup.ts) so they follow the consistent pattern.
In `@packages/uniwind/tests/media-queries/combined.test.tsx`:
- Around line 25-28: Replace the eval(...) call that constructs and runs the
reinit code for Uniwind.__reinit with a new Function(...) invocation (so you no
longer use eval) and ensure you add the same eslint-disable comment pattern used
elsewhere (see functions.ts/common.ts/serialize.ts) immediately above the new
Function call to suppress the linter rule for creating functions dynamically;
keep the Uniwind.__reinit(...) invocation and the virtualCode interpolation
logic intact while swapping the eval to new Function to avoid eval usage.
In `@packages/uniwind/tests/media-queries/max-width.test.tsx`:
- Around line 26-29: Replace the eval(...) wrapper around the string that
invokes Uniwind.__reinit by creating a new Function that takes the runtime (rt)
and the generated virtualCode and invokes Uniwind.__reinit; specifically, stop
calling eval with the template string and instead use the Function constructor
to build a callable function that receives rt and virtualCode and executes the
same body, then call that function with rt and the virtualCode variable so
Uniwind.__reinit is invoked without using eval.
In `@packages/uniwind/tests/media-queries/orientation.test.tsx`:
- Around line 14-30: Replace the global eval usage in beforeAll by providing a
real function initializer to Uniwind.__reinit instead of eval'ing the string in
virtualCode: change compileVirtual (or its caller) to return a callable (e.g., a
function that accepts rt and returns the virtual object) rather than JS source,
then call Uniwind.__reinit(initializerFn, ['light','dark']) directly; update the
test to require('../../src/core/config/config.native') to get Uniwind and pass
the returned function (not a string) into Uniwind.__reinit, removing the eval
and satisfying lint/security/noGlobalEval while keeping the same behavior for
virtualCode and themes.
♻️ Duplicate comments (2)
packages/uniwind/tests/media-queries/variables.test.tsx (1)
14-30: Duplicate: globalevalin test setup.Same issue as in
packages/uniwind/tests/media-queries/orientation.test.tsx; please apply the same refactor or suppression here.packages/uniwind/tests/media-queries/min-width.test.tsx (1)
14-30: Duplicate: globalevalin test setup.Same issue as in
packages/uniwind/tests/media-queries/orientation.test.tsx; please apply the same refactor or suppression here.
| eval( | ||
| `const { Uniwind } = require('../../src/core/config/config.native'); | ||
| Uniwind.__reinit(rt => ${virtualCode}, ['light', 'dark']); | ||
| `, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/uniwind/tests/media-queries && wc -l color-scheme.test.tsxRepository: uni-stack/uniwind
Length of output: 85
🏁 Script executed:
cat -n packages/uniwind/tests/media-queries/color-scheme.test.tsx | head -50Repository: uni-stack/uniwind
Length of output: 2334
🏁 Script executed:
# Check if there's a Biome configuration in the repo
find . -name "biome.json" -o -name ".biomeignore" -o -name "biome.jsonc" | head -5Repository: uni-stack/uniwind
Length of output: 73
🏁 Script executed:
# Search for eval usage patterns in the test file and check context
rg "eval\(" packages/uniwind/tests/media-queries/color-scheme.test.tsx -A 5 -B 5Repository: uni-stack/uniwind
Length of output: 375
🏁 Script executed:
cat biome.jsonRepository: uni-stack/uniwind
Length of output: 1835
🏁 Script executed:
# Check for security/eval rules in Biome config
rg "noEval|eval|security" biome.json -A 3 -B 3Repository: uni-stack/uniwind
Length of output: 43
🏁 Script executed:
# Check the actual Uniwind.__reinit signature to understand if the proposed fix is compatible
rg "__reinit" packages/uniwind/src -A 10 -B 2Repository: uni-stack/uniwind
Length of output: 5015
🏁 Script executed:
# Check if Biome's recommended rules include noEval
rg "noEval|eval" . --type json | head -20Repository: uni-stack/uniwind
Length of output: 43
🏁 Script executed:
# Verify the actual type signature of GenerateStyleSheetsCallback
rg "GenerateStyleSheetsCallback" packages/uniwind/src -B 2 -A 5Repository: uni-stack/uniwind
Length of output: 5399
🏁 Script executed:
# Check test setup to understand what virtualCode contains
cd packages/uniwind && rg "compileVirtual" src -A 3 -B 1Repository: uni-stack/uniwind
Length of output: 1118
🏁 Script executed:
# Check Biome's documentation for noEval rule behavior with new Function
web_search "Biome noEval rule new Function"Repository: uni-stack/uniwind
Length of output: 111
🏁 Script executed:
# Verify what virtualCode actually contains by checking compileVirtual return type
cat -n packages/uniwind/src/metro/compileVirtual.ts | head -100Repository: uni-stack/uniwind
Length of output: 3037
🏁 Script executed:
# Look for other test patterns in the codebase to see how they handle Uniwind.__reinit
rg "__reinit" packages/uniwind/tests -B 5 -A 5Repository: uni-stack/uniwind
Length of output: 6109
🏁 Script executed:
# Check Biome's recommended rules documentation by looking at their website
# For now, let's verify if there are any lint errors currently
rg "noEval" . --type jsonRepository: uni-stack/uniwind
Length of output: 43
🏁 Script executed:
# Let's verify the proposed fix by checking if new Function is also commonly flagged
rg "new Function" packages/uniwind -B 3 -A 3Repository: uni-stack/uniwind
Length of output: 2416
🏁 Script executed:
# Check if this eval pattern appears in production code or only in tests
rg "eval\(" packages/uniwind/src -B 2 -A 2Repository: uni-stack/uniwind
Length of output: 43
🏁 Script executed:
# Verify that virtualCode doesn't contain any external variable references
# by looking at what serializeJSObject returns
cat -n packages/uniwind/src/metro/utils.ts | head -50Repository: uni-stack/uniwind
Length of output: 128
🏁 Script executed:
# Check if there's any Biome-specific comment/ignore for eval in the codebase
rg "biome" packages/uniwind/tests -iRepository: uni-stack/uniwind
Length of output: 43
🏁 Script executed:
# Let's verify the exact structure of what virtualCode produces by looking at a real test output
# Check what the expected object structure is
rg "GenerateStyleSheetsCallback" packages/uniwind/src/core/types.ts -A 10Repository: uni-stack/uniwind
Length of output: 417
🏁 Script executed:
# Verify if the proposed new Function approach would maintain scope correctly
# by checking the actual signature expected
cat -n packages/uniwind/src/core/types.ts | head -50Repository: uni-stack/uniwind
Length of output: 1776
🏁 Script executed:
# Search for ESLint/Biome disable comments related to eval or function
rg "eslint-disable|biome-disable" packages/uniwind -iRepository: uni-stack/uniwind
Length of output: 1871
🏁 Script executed:
# Verify that new Function is also flagged by checking if disable comments are necessary
# by looking at whether Biome has a rule equivalent to ESLint's no-new-func
web_search Biome linter security rules eval new Function recommendedRepository: uni-stack/uniwind
Length of output: 111
🏁 Script executed:
# Check if there are any test utilities that could help avoid eval/new Function entirely
rg "renderUniwind\|UniwindListener\|UniwindStore" packages/uniwind/tests -l | head -5Repository: uni-stack/uniwind
Length of output: 43
🏁 Script executed:
# Verify if the pattern requires a function factory or if there's a way to directly set the callback
cat -n packages/uniwind/src/core/native/store.ts | sed -n '1,100p'Repository: uni-stack/uniwind
Length of output: 4389
🏁 Script executed:
# Check what the actual Biome/ESLint config expects for new Function usage
rg "no-new-func|no-implied-eval" packages/uniwind/.eslintrc -A 2 -B 2 2>/dev/null || echo "No .eslintrc found"Repository: uni-stack/uniwind
Length of output: 79
🏁 Script executed:
# Verify if .eslintignore exists for tests
find packages/uniwind -name ".eslintignore" -o -name ".eslintrc*" | xargs cat 2>/dev/null | head -30Repository: uni-stack/uniwind
Length of output: 43
🏁 Script executed:
# Check if there's a tsconfig or jest config that might affect linting
cat packages/uniwind/jest.config.js 2>/dev/null | head -50 || cat packages/uniwind/tsconfig.json 2>/dev/null | head -30Repository: uni-stack/uniwind
Length of output: 418
🏁 Script executed:
# Check if there are Biome ignore comment examples in the codebase
rg "biome-ignore" packages/uniwindRepository: uni-stack/uniwind
Length of output: 43
🏁 Script executed:
# Verify if Biome is actually being used for linting these test files
rg "ignorePatterns\|\"test\"\|\"tests\"" biome.jsonRepository: uni-stack/uniwind
Length of output: 43
Use new Function() instead of eval() to avoid linting failure.
The proposed fix is functionally correct, but requires linting disable comments to match the production code pattern. Update to:
🔧 Proposed fix (with linting disables)
+ // eslint-disable-next-line `@typescript-eslint/no-implied-eval`, no-new-func
+ const { Uniwind } = require('../../src/core/config/config.native')
+ const factory = new Function('rt', `return ${virtualCode}`)
+ Uniwind.__reinit(factory, ['light', 'dark'])
- eval(
- `const { Uniwind } = require('../../src/core/config/config.native');
- Uniwind.__reinit(rt => ${virtualCode}, ['light', 'dark']);
- `,
- )Note: This pattern appears in 6 other test files (variables, orientation, min-width, max-width, combined, setup.ts) that should be updated consistently.
🧰 Tools
🪛 Biome (2.1.2)
[error] 26-26: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
🤖 Prompt for AI Agents
In `@packages/uniwind/tests/media-queries/color-scheme.test.tsx` around lines 26 -
29, Replace the eval(...) call that invokes Uniwind.__reinit with a new
Function(...) pattern and add the same linting disable/enable comments used in
production tests so the test passes linting; specifically change the eval
invocation that builds and executes the runtime string for Uniwind.__reinit(rt
=> ${virtualCode}, ['light','dark']) to use new Function(...) to return and call
the function, and wrap that call with the same eslint-disable/enable comments
used elsewhere in tests; apply the identical change to the other test files
noted (variables, orientation, min-width, max-width, combined, setup.ts) so they
follow the consistent pattern.
| eval( | ||
| `const { Uniwind } = require('../../src/core/config/config.native'); | ||
| Uniwind.__reinit(rt => ${virtualCode}, ['light', 'dark']); | ||
| `, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "combined.test.tsx" --type fRepository: uni-stack/uniwind
Length of output: 115
🏁 Script executed:
if [ -f "packages/uniwind/tests/media-queries/combined.test.tsx" ]; then
wc -l "packages/uniwind/tests/media-queries/combined.test.tsx"
fiRepository: uni-stack/uniwind
Length of output: 119
🏁 Script executed:
cat -n "packages/uniwind/tests/media-queries/combined.test.tsx" | head -50Repository: uni-stack/uniwind
Length of output: 2361
🏁 Script executed:
rg "eval\(" packages/uniwind/tests --type tsx -B 2 -A 2Repository: uni-stack/uniwind
Length of output: 88
🏁 Script executed:
cat -n "packages/uniwind/tests/media-queries/combined.test.tsx" | sed -n '12,30p'Repository: uni-stack/uniwind
Length of output: 835
🏁 Script executed:
# Check what compileVirtual returns and how Uniwind.__reinit is used
rg "compileVirtual" packages/uniwind/tests --type tsx -B 1 -A 1 | head -40Repository: uni-stack/uniwind
Length of output: 88
🏁 Script executed:
# Look at other test files to understand the pattern
fd "\.test\.tsx$" packages/uniwind/tests --type fRepository: uni-stack/uniwind
Length of output: 2215
🏁 Script executed:
grep -r "eval(" packages/uniwind/tests --include="*.tsx" -B 2 -A 2Repository: uni-stack/uniwind
Length of output: 2843
🏁 Script executed:
# Check if there are other similar patterns in different test files
grep -r "__reinit" packages/uniwind/tests --include="*.tsx" -B 3 -A 3Repository: uni-stack/uniwind
Length of output: 3625
🏁 Script executed:
# Examine the config.native file to understand __reinit signature
cat -n "packages/uniwind/src/core/config/config.native.ts" | head -60Repository: uni-stack/uniwind
Length of output: 2574
🏁 Script executed:
# Check for Function constructor usage in the codebase
grep -r "new Function" packages/uniwind --include="*.ts" --include="*.tsx" -B 1 -A 1Repository: uni-stack/uniwind
Length of output: 1238
🏁 Script executed:
grep -A 10 "__reinit" packages/uniwind/src/core/config/config.native.ts | head -20Repository: uni-stack/uniwind
Length of output: 474
🏁 Script executed:
# Get more context about __reinit signature
grep -B 5 -A 15 "reinit.*=" packages/uniwind/src/core/config/config.native.ts | head -40Repository: uni-stack/uniwind
Length of output: 43
🏁 Script executed:
# Let's look at the full __reinit method
rg "__reinit" packages/uniwind/src/core/config --type ts -B 2 -A 10Repository: uni-stack/uniwind
Length of output: 2014
🏁 Script executed:
# Check what compileVirtual returns - is it an expression or statements?
cat -n packages/uniwind/src/metro/compileVirtual.ts | head -40Repository: uni-stack/uniwind
Length of output: 1602
🏁 Script executed:
# Verify the return type of compileVirtual
rg "compileVirtual.*=" packages/uniwind/src/metro --type ts -B 2 -A 5 | head -30Repository: uni-stack/uniwind
Length of output: 709
🏁 Script executed:
# Check if require can be called inside beforeAll (verify the proposed fix viability)
rg "beforeAll" packages/uniwind/tests -A 10 --type js | grep "require" | head -5Repository: uni-stack/uniwind
Length of output: 43
🏁 Script executed:
cat -n packages/uniwind/src/metro/compileVirtual.ts | tail -30Repository: uni-stack/uniwind
Length of output: 1369
🏁 Script executed:
# Look at the full function to understand return type
sed -n '21,80p' packages/uniwind/src/metro/compileVirtual.tsRepository: uni-stack/uniwind
Length of output: 1905
🏁 Script executed:
# Check how the virtualCode is being used - is it a function body or complete code?
cat -n "packages/uniwind/tests/media-queries/combined.test.tsx" | sed -n '13,30p'Repository: uni-stack/uniwind
Length of output: 769
🏁 Script executed:
# Verify the proposed fix by checking the GenerateStyleSheetsCallback type signature
rg "GenerateStyleSheetsCallback" packages/uniwind/src/core --type ts -B 2 -A 2Repository: uni-stack/uniwind
Length of output: 3758
🏁 Script executed:
# Test if the proposed new Function approach would work with the compiled virtualCode
python3 << 'EOF'
# Simulate what compileVirtual returns and what both eval and new Function would produce
virtualCode = '({ scopedVars: ({}), vars: ({}), stylesheet: ({}) })'
# Original eval approach (simulated)
eval_code = f"rt => {virtualCode}"
print("Eval creates arrow function: rt => " + virtualCode)
print()
# Proposed new Function approach
proposed_code = f"return {virtualCode}"
print("new Function creates function with body: " + proposed_code)
print()
# Both should create functions that take 'rt' param and return the same object
print("Both approaches create callable functions with signature: (rt) => object")
print("Semantics are equivalent ✓")
EOFRepository: uni-stack/uniwind
Length of output: 147
🏁 Script executed:
# Check if there are linting configurations that flag eval
find packages/uniwind -name "eslint*" -o -name ".eslintrc*" | xargs cat 2>/dev/null | grep -A 5 -B 5 "eval" || echo "No eval-specific eslint rules found in basic search"Repository: uni-stack/uniwind
Length of output: 112
🏁 Script executed:
# Verify similar patterns exist in other test files to confirm pattern
cat -n packages/uniwind/tests/media-queries/color-scheme.test.tsx | sed -n '24,30p'Repository: uni-stack/uniwind
Length of output: 308
Avoid eval() here as well; same lint/security concern.
The proposed fix with new Function() is correct, but it needs the eslint-disable comment to match the pattern used elsewhere in the codebase (see functions.ts, common.ts, serialize.ts).
🔧 Proposed fix (no eval)
- eval(
- `const { Uniwind } = require('../../src/core/config/config.native');
- Uniwind.__reinit(rt => ${virtualCode}, ['light', 'dark']);
- `,
- )
+ const { Uniwind } = require('../../src/core/config/config.native')
+ // eslint-disable-next-line `@typescript-eslint/no-implied-eval`, no-new-func
+ const factory = new Function('rt', `return ${virtualCode}`)
+ Uniwind.__reinit(factory, ['light', 'dark'])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| eval( | |
| `const { Uniwind } = require('../../src/core/config/config.native'); | |
| Uniwind.__reinit(rt => ${virtualCode}, ['light', 'dark']); | |
| `, | |
| const { Uniwind } = require('../../src/core/config/config.native') | |
| // eslint-disable-next-line `@typescript-eslint/no-implied-eval`, no-new-func | |
| const factory = new Function('rt', `return ${virtualCode}`) | |
| Uniwind.__reinit(factory, ['light', 'dark']) |
🧰 Tools
🪛 Biome (2.1.2)
[error] 25-25: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
🤖 Prompt for AI Agents
In `@packages/uniwind/tests/media-queries/combined.test.tsx` around lines 25 - 28,
Replace the eval(...) call that constructs and runs the reinit code for
Uniwind.__reinit with a new Function(...) invocation (so you no longer use eval)
and ensure you add the same eslint-disable comment pattern used elsewhere (see
functions.ts/common.ts/serialize.ts) immediately above the new Function call to
suppress the linter rule for creating functions dynamically; keep the
Uniwind.__reinit(...) invocation and the virtualCode interpolation logic intact
while swapping the eval to new Function to avoid eval usage.
| eval( | ||
| `const { Uniwind } = require('../../src/core/config/config.native'); | ||
| Uniwind.__reinit(rt => ${virtualCode}, ['light', 'dark']); | ||
| `, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/uniwind/tests/media-queries/max-width.test.tsx | sed -n '20,35p'Repository: uni-stack/uniwind
Length of output: 666
🏁 Script executed:
fd -t f -e json -e js -e yml -e yaml "eslint|biome|lint" | head -20Repository: uni-stack/uniwind
Length of output: 71
🏁 Script executed:
git ls-files | grep -E "(eslint|biome|lint)" | head -20Repository: uni-stack/uniwind
Length of output: 94
🏁 Script executed:
cat biome.jsonRepository: uni-stack/uniwind
Length of output: 1835
🏁 Script executed:
cat packages/uniwind/eslint.config.tsRepository: uni-stack/uniwind
Length of output: 1180
🏁 Script executed:
rg -t ts "eval" packages/uniwind/tests/media-queries/max-width.test.tsx -B 3 -A 3Repository: uni-stack/uniwind
Length of output: 283
🏁 Script executed:
git ls-files | grep -E "\.(yml|yaml|json)" | xargs grep -l "biome\|lint" | head -10Repository: uni-stack/uniwind
Length of output: 171
🏁 Script executed:
cat .github/workflows/ci.yamlRepository: uni-stack/uniwind
Length of output: 860
🏁 Script executed:
cat package.json | grep -A 20 '"scripts"'Repository: uni-stack/uniwind
Length of output: 729
🏁 Script executed:
cat packages/uniwind/package.json | grep -A 20 '"scripts"'Repository: uni-stack/uniwind
Length of output: 710
🏁 Script executed:
rg "biome" .github/workflows/ turbo.json package.jsonRepository: uni-stack/uniwind
Length of output: 43
Replace eval() with Function constructor for improved security and readability.
The eval() call is not flagged by the current linting setup (tests directory is excluded from ESLint), but removing it is still a best practice that reduces security risks and improves maintainability.
🔧 Proposed fix (no eval)
- eval(
- `const { Uniwind } = require('../../src/core/config/config.native');
- Uniwind.__reinit(rt => ${virtualCode}, ['light', 'dark']);
- `,
- )
+ const { Uniwind } = require('../../src/core/config/config.native')
+ const factory = new Function('rt', `return ${virtualCode}`)
+ Uniwind.__reinit(factory, ['light', 'dark'])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| eval( | |
| `const { Uniwind } = require('../../src/core/config/config.native'); | |
| Uniwind.__reinit(rt => ${virtualCode}, ['light', 'dark']); | |
| `, | |
| const { Uniwind } = require('../../src/core/config/config.native') | |
| const factory = new Function('rt', `return ${virtualCode}`) | |
| Uniwind.__reinit(factory, ['light', 'dark']) |
🧰 Tools
🪛 Biome (2.1.2)
[error] 26-26: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
🤖 Prompt for AI Agents
In `@packages/uniwind/tests/media-queries/max-width.test.tsx` around lines 26 -
29, Replace the eval(...) wrapper around the string that invokes
Uniwind.__reinit by creating a new Function that takes the runtime (rt) and the
generated virtualCode and invokes Uniwind.__reinit; specifically, stop calling
eval with the template string and instead use the Function constructor to build
a callable function that receives rt and virtualCode and executes the same body,
then call that function with rt and the virtualCode variable so Uniwind.__reinit
is invoked without using eval.
| beforeAll(async () => { | ||
| const cssPath = resolve('./tests/media-queries/orientation.test.css') | ||
| const css = readFileSync(cssPath, 'utf-8') | ||
| const virtualCode = await compileVirtual({ | ||
| css, | ||
| cssPath, | ||
| debug: true, | ||
| platform: Platform.iOS, | ||
| themes: ['light', 'dark'], | ||
| polyfills: undefined, | ||
| }) | ||
|
|
||
| eval( | ||
| `const { Uniwind } = require('../../src/core/config/config.native'); | ||
| Uniwind.__reinit(rt => ${virtualCode}, ['light', 'dark']); | ||
| `, | ||
| ) |
There was a problem hiding this comment.
Remove global eval to satisfy lint/security rules.
Biome flags this as lint/security/noGlobalEval, which can fail CI. Consider constructing the initializer without global eval. One option is to compile a function that accepts rt and returns the virtual object.
🔧 Suggested refactor (avoid global eval)
- eval(
- `const { Uniwind } = require('../../src/core/config/config.native');
- Uniwind.__reinit(rt => ${virtualCode}, ['light', 'dark']);
- `,
- )
+ const { Uniwind } = require('../../src/core/config/config.native')
+ const buildVirtual = new Function('rt', `return ${virtualCode}`)
+ Uniwind.__reinit(rt => buildVirtual(rt), ['light', 'dark'])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| beforeAll(async () => { | |
| const cssPath = resolve('./tests/media-queries/orientation.test.css') | |
| const css = readFileSync(cssPath, 'utf-8') | |
| const virtualCode = await compileVirtual({ | |
| css, | |
| cssPath, | |
| debug: true, | |
| platform: Platform.iOS, | |
| themes: ['light', 'dark'], | |
| polyfills: undefined, | |
| }) | |
| eval( | |
| `const { Uniwind } = require('../../src/core/config/config.native'); | |
| Uniwind.__reinit(rt => ${virtualCode}, ['light', 'dark']); | |
| `, | |
| ) | |
| beforeAll(async () => { | |
| const cssPath = resolve('./tests/media-queries/orientation.test.css') | |
| const css = readFileSync(cssPath, 'utf-8') | |
| const virtualCode = await compileVirtual({ | |
| css, | |
| cssPath, | |
| debug: true, | |
| platform: Platform.iOS, | |
| themes: ['light', 'dark'], | |
| polyfills: undefined, | |
| }) | |
| const { Uniwind } = require('../../src/core/config/config.native') | |
| const buildVirtual = new Function('rt', `return ${virtualCode}`) | |
| Uniwind.__reinit(rt => buildVirtual(rt), ['light', 'dark']) |
🧰 Tools
🪛 Biome (2.1.2)
[error] 26-26: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
🤖 Prompt for AI Agents
In `@packages/uniwind/tests/media-queries/orientation.test.tsx` around lines 14 -
30, Replace the global eval usage in beforeAll by providing a real function
initializer to Uniwind.__reinit instead of eval'ing the string in virtualCode:
change compileVirtual (or its caller) to return a callable (e.g., a function
that accepts rt and returns the virtual object) rather than JS source, then call
Uniwind.__reinit(initializerFn, ['light','dark']) directly; update the test to
require('../../src/core/config/config.native') to get Uniwind and pass the
returned function (not a string) into Uniwind.__reinit, removing the eval and
satisfying lint/security/noGlobalEval while keeping the same behavior for
virtualCode and themes.
|
As mentioned in the linked issue #242 , we are currently heads-down on ScopedThemes and won't be able to give this the thorough review. To keep our active PR queue manageable and focused on the current release, I’m going to go ahead and close this for now. Please note: This is not a rejection. We simply close PRs that are on hold to keep our workspace clean. |
2 participants
fixes #242
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.