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: Problem with stub props #610

Merged
merged 10 commits into from
May 25, 2021

Conversation

YutamaKotaro
Copy link
Contributor

@YutamaKotaro YutamaKotaro commented May 22, 2021

For what ??

This pull request is for #530

why the issue happened ?

CreateStub was using props of transformVNodeArgs on renderFunction.

return h(tag, props, renderStubDefaultSlot ? ctx.$slots : undefined)<br class="Apple-interchange-newline">

This code means that the firstProps is always used for render function.
Hence, if we use shallow or stub this, problem happened.

@@ -119,7 +119,7 @@ describe('mounting options: stubs', () => {

expect(wrapper.html()).toEqual(
'<div>\n' +
' <foo-stub class="bar" test-id="foo" dynamic="[object Object]"></foo-stub>\n' +
' <foo-stub dynamic="[object Object]" class="bar" test-id="foo"></foo-stub>\n' +
Copy link
Contributor Author

@YutamaKotaro YutamaKotaro May 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Order of props and attributes is changed .....this is my concern...
If we need to keep this order... I'll try it !!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order of attributes has no specific meaning, so it's fine if it changes a bit. This is one of the things I dislike about snapshots; they often change, even if the component works the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got it !!
Thanks a lot !!

Copy link
Member

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code seems fine - shallowMount is so complicated, lol.

You mention a problem - is that the order of the attributes? I don't think this is really a problem - the order isn't important for the actual behavior. Some people might need to update their snapshots. That's fine - the order of the props is not guaranteed to be deterministic, as far as I know.

@@ -119,7 +119,7 @@ describe('mounting options: stubs', () => {

expect(wrapper.html()).toEqual(
'<div>\n' +
' <foo-stub class="bar" test-id="foo" dynamic="[object Object]"></foo-stub>\n' +
' <foo-stub dynamic="[object Object]" class="bar" test-id="foo"></foo-stub>\n' +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order of attributes has no specific meaning, so it's fine if it changes a bit. This is one of the things I dislike about snapshots; they often change, even if the component works the same.

propsDeclaration,
renderStubDefaultSlot
}: StubOptions): ComponentOptions => {
const anonName = 'anonymous-stub'
const tag = name ? `${hyphenate(name)}-stub` : anonName

const render = (ctx: ComponentPublicInstance) => {
return h(tag, props, renderStubDefaultSlot ? ctx.$slots : undefined)
return h(tag, ctx.$props, renderStubDefaultSlot ? ctx.$slots : undefined)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, nice.

src/stubs.ts Outdated
@@ -112,6 +111,7 @@ export function stubComponents(
shallow: boolean = false,
renderStubDefaultSlot: boolean = false
) {
const component: { [key: string]: ComponentOptions } = {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could write Record<string, ComponentOptions>.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got it !! I'll fix it soon !!

@YutamaKotaro
Copy link
Contributor Author

Code seems fine - shallowMount is so complicated, lol.

Yep, I think so too. It looks me a little time to find problems lol.

You mention a problem - is that the order of the attributes? I don't think this is really a problem - the order isn't important for the actual behavior. Some people might need to update their snapshots. That's fine - the order of the props is not guaranteed to be deterministic, as far as I know.

That's right. What I meant was the order of the attributes.
Thanks a lot !! I'm relived to hear of that.

@YutamaKotaro
Copy link
Contributor Author

I corrected the points you pointed out !!

@lmiller1990 lmiller1990 self-requested a review May 25, 2021 01:54
@lmiller1990 lmiller1990 merged commit 279aa83 into vuejs:master May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants