Skip to content

Commit

Permalink
[Primitives]fix: bug props not inheritted correctly (#242)
Browse files Browse the repository at this point in the history
* fix props not inheritted correctly

* write test for primitive
  • Loading branch information
zernonia authored Jul 25, 2023
1 parent ba1dba1 commit 83af804
Show file tree
Hide file tree
Showing 2 changed files with 172 additions and 18 deletions.
141 changes: 141 additions & 0 deletions packages/radix-vue/src/Primitive/Primitive.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import { mount } from "@vue/test-utils";
import { PrimitiveDiv } from "./";
import { describe, expect, it } from "vitest";

describe("test Primitive functionalities", () => {
it("should render div element correctly", () => {
const wrapper = mount(PrimitiveDiv);
expect(wrapper.find("div").exists()).toBe(true);
});

it("should renders div element with custom attribute", () => {
const wrapper = mount(PrimitiveDiv, {
attrs: {
type: "button",
},
});

const element = wrapper.find("div");

expect(element.attributes("type")).toBe("button");
});

it("should renders multiple child elements", () => {
const wrapper = mount(PrimitiveDiv, {
slots: {
default: "<div>1</div><div>2</div><div>3</div>",
},
});

const element = wrapper.find("div");
expect(element.findAll("div").length).toBe(3);
});

// ref: https://vitest.dev/api/expect.html#tothrowerror
describe("asChild", () => {
it("should throw error when multiple child elements exists", () => {
const wrapper = () =>
mount(PrimitiveDiv, {
props: {
asChild: true,
},
slots: {
default: "<div>1</div><div>2</div><div>3</div>",
},
});

expect(() => wrapper()).toThrowError(/invalid children/);
});

it("should merge child's class together", () => {
const wrapper = mount(PrimitiveDiv, {
props: {
asChild: true,
},
attrs: {
class: "parent-class",
},
slots: {
default:
'<div class="child-class more-child-class">Child class</div>',
},
});

const element = wrapper.find("div");
expect(element.attributes("class")).toBe(
"parent-class child-class more-child-class"
);
});

it("should render the child class element tag", () => {
const wrapper = mount(PrimitiveDiv, {
props: {
asChild: true,
},

slots: {
default: "<a>Child class</a>",
},
});

const element = wrapper.find("a");
expect(element.exists()).toBeTruthy();
});

it("should render the child component", () => {
const ChildComponent = {
template: '<div id="child">Hello world</div>',
};
const RootComponent = {
components: { ChildComponent, PrimitiveDiv },
template: "<PrimitiveDiv><ChildComponent /></PrimitiveDiv>",
};

const wrapper = mount(RootComponent, {
props: {
asChild: true,
},
});

const element = wrapper.find("div");
expect(element.html()).toBe('<div id="child">Hello world</div>');
});

it("should inherit parent attributes and the child attributes", () => {
const wrapper = mount(PrimitiveDiv, {
props: {
asChild: true,
},
attrs: {
"data-parent-attr": "",
},
slots: {
default: "<div data-child-attr>Child class</div>",
},
});

const element = wrapper.find("div");
expect(element.attributes("data-parent-attr")).toBe("");
expect(element.attributes("data-child-attr")).toBe("");
});

it("should replace parent attributes with child's attributes", () => {
const wrapper = mount(PrimitiveDiv, {
props: {
asChild: true,
},
attrs: {
id: "parent",
"data-type": "button",
},
slots: {
default: '<div id="child" data-type="primary">Child class</div>',
},
});

const element = wrapper.find("div");
expect(element.attributes("data-type")).toBe("primary");
expect(element.attributes("id")).toBe("child");
});
});
});
49 changes: 31 additions & 18 deletions packages/radix-vue/src/Primitive/Primitive.vue
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
getCurrentInstance,
mergeProps,
cloneVNode,
type ComponentInternalInstance,
} from "vue";
import { renderSlotFragments, isValidVNodeElement } from "@/shared";
Expand All @@ -28,8 +29,30 @@ const NODES = [
"ul",
] as const;
const throwError = (instance: ComponentInternalInstance | null) => {
const componentName = instance?.parent?.type.name
? `<${instance.parent.type.name} />`
: "component";
throw new Error(
[
`Detected an invalid children for \`${componentName}\` with \`asChild\` prop.`,
"",
"Note: All components accepting `asChild` expect only one direct child of valid VNode type.",
"You can apply a few solutions:",
[
"Provide a single child element so that we can forward the props onto that element.",
"Ensure the first child is an actual element instead of a raw text node or comment node.",
]
.map((line) => ` - ${line}`)
.join("\n"),
].join("\n")
);
};
const createComponent = (node: (typeof NODES)[number]) =>
defineComponent({
inheritAttrs: false,
props: {
asChild: {
type: Boolean,
Expand All @@ -54,28 +77,16 @@ const createComponent = (node: (typeof NODES)[number]) =>
if (Object.keys(attrs).length > 0) {
const [firstChild, ...otherChildren] = children;
if (!isValidVNodeElement(firstChild) || otherChildren.length > 0) {
const componentName = instance?.parent?.type.name
? `<${instance.parent.type.name} />`
: "component";
throw new Error(
[
`Detected an invalid children for \`${componentName}\` with \`asChild\` prop.`,
"",
"Note: All components accepting `asChild` expect only one direct child of valid VNode type.",
"You can apply a few solutions:",
[
"Provide a single child element so that we can forward the props onto that element.",
"Ensure the first child is an actual element instead of a raw text node or comment node.",
]
.map((line) => ` - ${line}`)
.join("\n"),
].join("\n")
);
throwError(instance);
}
// remove props ref from being inferred
delete firstChild.props?.ref;
const mergedProps = mergeProps(firstChild.props ?? {}, attrs);
const mergedProps = mergeProps(attrs, firstChild.props ?? {});
// remove class to prevent duplicated
delete firstChild.props?.class;
const cloned = cloneVNode(firstChild, mergedProps);
// Explicitly override props starting with `on`.
// It seems cloneVNode from Vue doesn't like overriding `onXXX` props. So
Expand All @@ -87,6 +98,8 @@ const createComponent = (node: (typeof NODES)[number]) =>
}
}
return cloned;
} else if (Array.isArray(children) && children.length > 1) {
throwError(instance);
} else if (Array.isArray(children) && children.length === 1) {
// No props to inherit
return children[0];
Expand Down

0 comments on commit 83af804

Please sign in to comment.