fix(runtime): generated <comp /> cannot use style#18877
fix(runtime): generated <comp /> cannot use style#18877rdjanuar wants to merge 2 commits intoNervJS:mainfrom
Conversation
功能概述本次PR在小程序示例中添加了样式隔离(style-isolation)相关功能,包括新增页面配置、组件、样式表和运行时DSL配置,同时补充了对应的单元测试以验证不同环境下的样式隔离行为。 变更详情
预估代码审查工作量🎯 2 (Simple) | ⏱️ ~12分钟 建议审查者
诗(兔子的祝福)
🚥 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
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Stylelint (17.3.0)examples/mini-program-example/src/pages/style-isolation/index.scssAllFilesIgnoredError: All input files were ignored because of the ignore pattern. Either change your input, ignore pattern or use "--allow-empty-input" to allow no inputs Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/taro-runtime/src/dsl/common.ts (1)
389-390: 可选:使用||替代.includes()以保持与jd分支的一致性当前写法使用了
as unknown as string双重类型断言,这是因为process.env.TARO_ENV被推断为字面量联合类型而非string。直接使用===不需要类型断言,且与上方jd的检查方式保持一致:♻️ 建议的重构
- } else if (['weapp', 'harmony-hybrid'].includes(process.env.TARO_ENV as unknown as string)) { + } else if (process.env.TARO_ENV === 'weapp' || process.env.TARO_ENV === 'harmony-hybrid') {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/taro-runtime/src/dsl/common.ts` around lines 389 - 390, The conditional that sets extraOptions.styleIsolation currently uses ['weapp', 'harmony-hybrid'].includes(process.env.TARO_ENV as unknown as string); change it to use explicit equality checks with || against process.env.TARO_ENV (e.g., process.env.TARO_ENV === 'weapp' || process.env.TARO_ENV === 'harmony-hybrid') to avoid the double type assertion and match the `jd` branch style—update the conditional that controls extraOptions.styleIsolation accordingly.examples/mini-program-example/src/pages/style-isolation/index.tsx (1)
5-54: 建议为深层嵌套添加注释以说明意图21 层嵌套是故意为之(用于复现 issue
#18851中的样式隔离问题),但缺少注释说明,后续维护者可能会困惑或"优化"掉这个嵌套层级,导致测试场景失效。建议在组件顶部或render方法中添加简短注释:💡 建议添加注释
+// 此页面用于复现深层嵌套场景下生成组件(<comp />)无法继承样式的问题 +// 见: https://github.com/NervJS/taro/issues/18851 +// 21 层嵌套是为了触发递归组件的样式隔离边界 export default class Index extends Component {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/mini-program-example/src/pages/style-isolation/index.tsx` around lines 5 - 54, Add a short explanatory comment in the Index component (class Index) — either at the top of the class or inside the render method — stating that the deep 21-level <View> nesting is intentional to reproduce the style-isolation regression (see issue `#18851`) and must not be flattened or removed; include a TODO/comment marker like "INTENTIONAL DEEP NESTING — do not refactor" and a reference to the issue number so future maintainers understand why the nesting exists (particularly the nested <Text className='red-text'>Level 21 - Should be RED</Text> assertion).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/mini-program-example/src/pages/style-isolation/index.scss`:
- Line 3: The font-size declaration in index.scss currently uses "40px" which
doesn't match the screenshot; update the unit to "40rpx" (replace the
"font-size: 40px" rule with "font-size: 40rpx") so the style uses the WeChat
Mini Program responsive unit; locate the rule in the style-isolation/index.scss
file and change the unit accordingly.
---
Nitpick comments:
In `@examples/mini-program-example/src/pages/style-isolation/index.tsx`:
- Around line 5-54: Add a short explanatory comment in the Index component
(class Index) — either at the top of the class or inside the render method —
stating that the deep 21-level <View> nesting is intentional to reproduce the
style-isolation regression (see issue `#18851`) and must not be flattened or
removed; include a TODO/comment marker like "INTENTIONAL DEEP NESTING — do not
refactor" and a reference to the issue number so future maintainers understand
why the nesting exists (particularly the nested <Text className='red-text'>Level
21 - Should be RED</Text> assertion).
In `@packages/taro-runtime/src/dsl/common.ts`:
- Around line 389-390: The conditional that sets extraOptions.styleIsolation
currently uses ['weapp', 'harmony-hybrid'].includes(process.env.TARO_ENV as
unknown as string); change it to use explicit equality checks with || against
process.env.TARO_ENV (e.g., process.env.TARO_ENV === 'weapp' ||
process.env.TARO_ENV === 'harmony-hybrid') to avoid the double type assertion
and match the `jd` branch style—update the conditional that controls
extraOptions.styleIsolation accordingly.
| @@ -0,0 +1,9 @@ | |||
| .red-text { | |||
| color: red; | |||
| font-size: 40px; | |||
There was a problem hiding this comment.
px 单位与截图不一致,建议改为 rpx
截图中开发者工具显示的是 font-size: 40rpx,而代码中为 40px。对于微信小程序,rpx 是推荐的响应式单位。
🛠️ 建议修改
- font-size: 40px;
+ font-size: 40rpx;📝 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.
| font-size: 40px; | |
| font-size: 40rpx; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/mini-program-example/src/pages/style-isolation/index.scss` at line
3, The font-size declaration in index.scss currently uses "40px" which doesn't
match the screenshot; update the unit to "40rpx" (replace the "font-size: 40px"
rule with "font-size: 40rpx") so the style uses the WeChat Mini Program
responsive unit; locate the rule in the style-isolation/index.scss file and
change the unit accordingly.
这个 PR 做了什么? (简要描述所做更改)
这个 PR 是什么类型? (至少选择一个)
这个 PR 涉及以下平台:
Summary by CodeRabbit
发布说明
新功能
测试