Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.8k
feat(compiler-dom): support customizable select#13616
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
coderabbitaibot commentedJul 11, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
WalkthroughThe changes update HTML nesting validation logic by removing specific child element restrictions for Changes
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat withCodeRabbit:
SupportNeed help? Create a ticket on oursupport page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File ( |
Size ReportBundles
Usages
|
@vue/compiler-core@vue/compiler-dom@vue/compiler-sfc@vue/compiler-ssr@vue/reactivity@vue/runtime-core@vue/runtime-dom@vue/server-renderer@vue/sharedvue@vue/compatcommit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/compiler-dom/__tests__/transforms/validateHtmlNesting.spec.ts(1 hunks)packages/compiler-dom/src/htmlNesting.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/compiler-dom/src/htmlNesting.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/compiler-dom/__tests__/transforms/validateHtmlNesting.spec.ts (1)
packages/compiler-dom/src/index.ts (1)
compile(41-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
| it("Don't warn with customize select and option", () => { | ||
| let err: CompilerError | undefined | ||
| compile(`<select><option></option></select>`, { onWarn: e => (err = e) }) | ||
| compile(`<option><p></p></option>`, { onWarn: e => (err = e) }) | ||
| expect(err).toBeUndefined() | ||
| }) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fix test logic and improve test structure.
The test has a potential issue where theerr variable is shared between two compile calls, which could lead to false test results. If the first compile call generates a warning but the second doesn't, the test would incorrectly pass.
Additionally, the test name has grammatical issues and should be corrected.
Apply this diff to fix the test logic and improve the structure:
- it("Don't warn with customize select and option", () => {+ it("should not warn with customizable select and option", () => { let err: CompilerError | undefined compile(`<select><option></option></select>`, { onWarn: e => (err = e) })+ expect(err).toBeUndefined()++ err = undefined // Reset for next test compile(`<option><p></p></option>`, { onWarn: e => (err = e) }) expect(err).toBeUndefined() })
Alternatively, consider splitting this into two separate test cases for better clarity and isolation:
- it("Don't warn with customize select and option", () => {- let err: CompilerError | undefined- compile(`<select><option></option></select>`, { onWarn: e => (err = e) })- compile(`<option><p></p></option>`, { onWarn: e => (err = e) })- expect(err).toBeUndefined()- })+ it('should not warn with customizable select', () => {+ let err: CompilerError | undefined+ compile(`<select><option></option></select>`, { onWarn: e => (err = e) })+ expect(err).toBeUndefined()+ })++ it('should not warn with customizable option', () => {+ let err: CompilerError | undefined+ compile(`<option><p></p></option>`, { onWarn: e => (err = e) })+ expect(err).toBeUndefined()+ })
📝 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.
| it("Don't warn with customize select and option",()=>{ | |
| leterr:CompilerError|undefined | |
| compile(`<select><option></option></select>`,{onWarn:e=>(err=e)}) | |
| compile(`<option><p></p></option>`,{onWarn:e=>(err=e)}) | |
| expect(err).toBeUndefined() | |
| }) | |
| it("should not warn with customizable select and option",()=>{ | |
| leterr:CompilerError|undefined | |
| compile(`<select><option></option></select>`,{onWarn:e=>(err=e)}) | |
| expect(err).toBeUndefined() | |
| err=undefined// Reset for next test | |
| compile(`<option><p></p></option>`,{onWarn:e=>(err=e)}) | |
| expect(err).toBeUndefined() | |
| }) |
🤖 Prompt for AI Agents
In packages/compiler-dom/__tests__/transforms/validateHtmlNesting.spec.ts aroundlines 14 to 20, the test shares the same error variable between two compilecalls, which can cause false positives if the first call sets an error but thesecond does not. To fix this, separate the tests into two distinct test caseseach with its own error variable to isolate warnings properly. Also, rename thetest to correct the grammar for clarity, for example, change "Don't warn withcustomize select and option" to "Does not warn with customized select andoption."
Uh oh!
There was an error while loading.Please reload this page.
close#13608
Summary by CodeRabbit
<select>with<option>elements and when nesting elements inside<option>.<select><option></option></select>and<option><p></p></option>structures.