feat: support conditional compilation layout#86
feat: support conditional compilation layout#86Wyatex wants to merge 1 commit intouni-helper:mainfrom
Conversation
📝 WalkthroughWalkthroughIntroduces a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/utils.ts`:
- Around line 101-136: Update preprocess to support uni-app conditional
directives by extending the comment-handling branch in function preprocess: add
handling for '#else' (flip the top of stack to the inverse of previous condition
evaluation for that block) and '#elif' (evaluate the new condition with
evaluate(..., context) and replace the top of the stack with that result,
ensuring only one active branch per conditional chain); ensure shouldInclude
logic remains stack.every(v => v) to respect nested results. Also add validation
after the loop to check if stack.length !== 0 and emit a clear warning or throw
an error (using console.warn/console.error or a provided logger) indicating
unclosed directives so unbalanced `#ifdef/`#endif pairs are not silently ignored.
Make sure to reference and use getPlatformContext and evaluate consistently and
preserve existing behavior of pushing blank lines into result for directive
lines.
🧹 Nitpick comments (3)
src/utils.ts (2)
47-63: JSDoc comment is slightly misleading.The comment mentions that
@dcloudio/uni-cli-shared'sparsePagesJsondoesn't support parsingsubPackage, but this function also doesn't directly handle subPackages—that logic remains inloadPagesJson. Consider clarifying that this function handles conditional compilation preprocessing, which is the actual feature being added.Also, the silent
return {}on catch may mask real JSON syntax errors in pages.json. Consider logging or propagating errors in development mode so users can debug malformed configurations.
86-99: Consider a safer alternative tonew Function()for expression evaluation.Using
new Function()to evaluate boolean expressions works but is a security smell and may trigger CSP violations in some environments. Since the supported operators appear to be limited to||and&&with identifiers, consider a simple recursive descent parser or explicit handling of these operators instead.♻️ Safer alternative without dynamic code execution
function evaluate(condition: string, context: Record<string, boolean>): boolean { - const code = condition.replace(/[a-zA-Z0-9_$-]+/g, (match) => { - if (match === 'true' || match === 'false') - return match - return context[match] ? 'true' : 'false' - }) - try { - // eslint-disable-next-line no-new-func - return new Function(`return !!(${code})`)() - } - catch { - return false - } + // Handle || operator (uni-app's primary compound operator) + if (condition.includes('||')) { + return condition.split('||').some(part => evaluate(part.trim(), context)) + } + // Handle && operator + if (condition.includes('&&')) { + return condition.split('&&').every(part => evaluate(part.trim(), context)) + } + // Single identifier lookup + const trimmed = condition.trim() + if (trimmed === 'true') return true + if (trimmed === 'false') return false + return !!context[trimmed] }Note: This simpler approach doesn't handle operator precedence or parentheses, but neither does the current implementation effectively. If complex expressions are needed, consider a proper expression parser.
test/loadPagesJson.test.ts (1)
11-114: Good test coverage for primary use cases.The tests effectively cover platform-specific inclusion, platform hierarchy, H5/WEB aliases, and the
||operator.Consider adding tests for these edge cases to improve robustness:
- Nested conditionals (
#ifdefinside#ifdef)- The
&&operator if intended to be supported- Empty platform string (the fallback case when
UNI_PLATFORMis unset)- Unbalanced/malformed directives to verify graceful handling
💡 Example additional test cases
it('nested ifdef', () => { const jsonString = `{ "pages": [ // `#ifdef` APP // `#ifdef` APP-PLUS { "path": "pages/app-plus-only" } // `#endif` // `#endif` ] }` expect(parsePagesJson(jsonString, 'APP-PLUS')).toEqual({ pages: [{ path: 'pages/app-plus-only' }], }) expect(parsePagesJson(jsonString, 'APP-ANDROID')).toEqual({ pages: [], }) }) it('empty platform defaults gracefully', () => { const jsonString = `{ "pages": [ // `#ifdef` H5 { "path": "pages/h5-only" } // `#endif` ] }` expect(parsePagesJson(jsonString, '')).toEqual({ pages: [], }) })
| function preprocess(content: string, platform: string) { | ||
| const context = getPlatformContext(platform) | ||
| const lines = content.split('\n') | ||
| const result: string[] = [] | ||
| const stack: boolean[] = [] | ||
|
|
||
| const shouldInclude = () => stack.every(v => v) | ||
|
|
||
| for (const line of lines) { | ||
| const trimmed = line.trim() | ||
| if (trimmed.startsWith('//')) { | ||
| const comment = trimmed.substring(2).trim() | ||
| if (comment.startsWith('#ifdef')) { | ||
| stack.push(evaluate(comment.substring(6).trim(), context)) | ||
| result.push('') | ||
| continue | ||
| } | ||
| if (comment.startsWith('#ifndef')) { | ||
| stack.push(!evaluate(comment.substring(7).trim(), context)) | ||
| result.push('') | ||
| continue | ||
| } | ||
| if (comment.startsWith('#endif')) { | ||
| stack.pop() | ||
| result.push('') | ||
| continue | ||
| } | ||
| } | ||
|
|
||
| if (shouldInclude()) | ||
| result.push(line) | ||
| else | ||
| result.push('') | ||
| } | ||
| return result.join('\n') | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
uni-app conditional compilation #else #elif directives syntax
💡 Result:
Yes — uni-app conditional compilation uses comment-style directives similar to C. Use // (JS), /* */ (CSS), or (Vue templates) and the directives #ifdef, #ifndef, #elif, #else and end with #endif.
Syntax (JS example):
// #ifdef APP-PLUS
// code for App
// #elif H5
// code for H5
// #else
// fallback code for other platforms
// #endif
Notes:
- Multiple platforms can be combined with || (e.g. APP-PLUS||H5).
- Use /* ... */ for styles and inside templates.
- Valid platform identifiers and supported files are listed in the uni-app docs. [1][2]
Sources:
[1] uni-app — Conditional compilation / Cross-platform docs.
[2] Community guide / examples showing #elif usage in uni-app.
🏁 Script executed:
# Check the actual implementation in src/utils.ts
cat -n src/utils.ts | sed -n '101,136p'Repository: uni-helper/vite-plugin-uni-layouts
Length of output: 1278
🏁 Script executed:
# Search for test files related to preprocess function
fd -e test.ts -e spec.ts -e test.js -e spec.js | head -20Repository: uni-helper/vite-plugin-uni-layouts
Length of output: 200
🏁 Script executed:
# Search for any existing `#else` or `#elif` handling in the codebase
rg -i '#else|#elif' --type ts --type jsRepository: uni-helper/vite-plugin-uni-layouts
Length of output: 60
🏁 Script executed:
# Search for other references to the preprocess function
rg 'preprocess' --type ts --type js -B 2 -A 2Repository: uni-helper/vite-plugin-uni-layouts
Length of output: 546
🏁 Script executed:
# Search for tests specifically for the preprocess function
rg 'preprocess' test/ --type ts --type js -B 5 -A 10Repository: uni-helper/vite-plugin-uni-layouts
Length of output: 60
Add support for #elif and #else directives to match uni-app's conditional compilation syntax.
The uni-app framework officially supports #elif and #else directives alongside #ifdef, #ifndef, and #endif. The current implementation only handles #ifdef, #ifndef, and #endif, which prevents developers from using the full conditional compilation feature set documented in uni-app's specs.
Additionally, unbalanced #ifdef/#endif pairs are silently ignored. Add validation to warn or error when stack is non-empty after processing all lines, indicating unclosed directives.
🤖 Prompt for AI Agents
In `@src/utils.ts` around lines 101 - 136, Update preprocess to support uni-app
conditional directives by extending the comment-handling branch in function
preprocess: add handling for '#else' (flip the top of stack to the inverse of
previous condition evaluation for that block) and '#elif' (evaluate the new
condition with evaluate(..., context) and replace the top of the stack with that
result, ensuring only one active branch per conditional chain); ensure
shouldInclude logic remains stack.every(v => v) to respect nested results. Also
add validation after the loop to check if stack.length !== 0 and emit a clear
warning or throw an error (using console.warn/console.error or a provided
logger) indicating unclosed directives so unbalanced `#ifdef/`#endif pairs are not
silently ignored. Make sure to reference and use getPlatformContext and evaluate
consistently and preserve existing behavior of pushing blank lines into result
for directive lines.
1 participant
Description 描述
原来的逻辑不支持
pages.json的条件编译去选择不同的layout，希望插件支持这个功能Linked Issues 关联的 Issues
#85
Additional context 额外上下文
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.