Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(runtime-core): the select tag's multiple prop should be set before the children mounting #3202

Merged
merged 3 commits into from
Mar 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions packages/runtime-core/src/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import {
Static,
VNodeNormalizedRef,
VNodeHook,
VNodeNormalizedRefAtom
VNodeNormalizedRefAtom,
VNodeProps
} from './vnode'
import {
ComponentInternalInstance,
Expand Down Expand Up @@ -114,7 +115,8 @@ export interface RendererOptions<
createElement(
type: string,
isSVG?: boolean,
isCustomizedBuiltIn?: string
isCustomizedBuiltIn?: string,
vnodeProps?: (VNodeProps & { [key: string]: any }) | null
): HostElement
createText(text: string): HostNode
createComment(text: string): HostNode
Expand Down Expand Up @@ -738,7 +740,8 @@ function baseCreateRenderer(
el = vnode.el = hostCreateElement(
vnode.type as string,
isSVG,
props && props.is
props && props.is,
props
)

// mount children first, since some props may rely on child content
Expand Down
18 changes: 17 additions & 1 deletion packages/runtime-dom/__tests__/nodeOps.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { nodeOps } from '../src/nodeOps'

describe('nodeOps', () => {
describe('runtime-dom: node-ops', () => {
test('the _value property should be cloned', () => {
const el = nodeOps.createElement('input') as HTMLDivElement & {
_value: any
Expand All @@ -9,4 +9,20 @@ describe('nodeOps', () => {
const cloned = nodeOps.cloneNode!(el) as HTMLDivElement & { _value: any }
expect(cloned._value).toBe(1)
})

test("the <select>'s multiple attr should be set in createElement", () => {
const el = nodeOps.createElement('select', false, undefined, {
multiple: ''
}) as HTMLSelectElement
const option1 = nodeOps.createElement('option') as HTMLOptionElement
const option2 = nodeOps.createElement('option') as HTMLOptionElement
option1.selected = true
option2.selected = true
nodeOps.insert(option1, el)
nodeOps.insert(option2, el)

expect(el.multiple).toBe(true)
expect(option1.selected).toBe(true)
expect(option2.selected).toBe(true)
})
})
13 changes: 10 additions & 3 deletions packages/runtime-dom/src/nodeOps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,17 @@ export const nodeOps: Omit<RendererOptions<Node, Element>, 'patchProp'> = {
}
},

createElement: (tag, isSVG, is): Element =>
isSVG
createElement: (tag, isSVG, is, props): Element => {
const el = isSVG
? doc.createElementNS(svgNS, tag)
: doc.createElement(tag, is ? { is } : undefined),
: doc.createElement(tag, is ? { is } : undefined)

if (tag === 'select' && props && props.multiple != null) {
;(el as HTMLSelectElement).setAttribute('multiple', props.multiple)
}

return el
},

createText: text => doc.createTextNode(text),

Expand Down