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): Error problem in custom instruction SSR environment #4273

Closed
wants to merge 7 commits into from

Conversation

Aaminly
Copy link

@Aaminly Aaminly commented Aug 8, 2021

// runtime-core directives.ts withDirectives
if (dir && dir.deep) {
    traverse(value)
  }
// runtime-core directives.ts invokeDirectiveHook
let hook = binding && binding.dir && binding.dir[name] as DirectiveHook | DirectiveHook[] | undefined
if (__COMPAT__ && !hook) {
  hook = mapCompatDirectiveHook(name, binding.dir, instance)
}
if (hook) {

image
image

Both dir and binding may be undefined, resulting in a deep error

@Aaminly Aaminly changed the title feat(runtime-core): Error problem in custom instruction SSR environment fix(runtime-core): Error problem in custom instruction SSR environment Aug 8, 2021
@@ -103,7 +103,7 @@ export function withDirectives<T extends VNode>(
updated: dir
} as ObjectDirective
}
if (dir.deep) {
if (dir && dir.deep) {
Copy link
Member

Choose a reason for hiding this comment

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

I think if dir is undefined, it should jump out of the current loop.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, we should really jump out of the loop. At present, such judgment and processing can also achieve the expected effect.

Copy link
Author

Choose a reason for hiding this comment

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

I don't have complete reading logic. Thank you for reminding me.

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

Can you add a failing test case that would pass with this change?

@@ -103,6 +103,7 @@ export function withDirectives<T extends VNode>(
updated: dir
} as ObjectDirective
}
if (!dir) continue;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (!dir) continue;
let [dir, value, arg, modifiers = EMPTY_OBJ] = directives[i]
if (!dir) continue;

@Aaminly
Copy link
Author

Aaminly commented Oct 3, 2021

The main problem is that in the SSR environment, if an instruction is written on the label but not declared, an error will be reported, and the error information is not intuitive enough

@skirtles-code
Copy link
Contributor

I believe this problem was fixed in #6671, so this PR can now be closed.

@edison1105 edison1105 closed this Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Rejected
Development

Successfully merging this pull request may close these issues.

None yet

5 participants