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(compiler): comment can cause vIf-related syntax errors #6843

Closed
wants to merge 2 commits into from

Conversation

Dunqing
Copy link
Contributor

@Dunqing Dunqing commented Oct 10, 2022

fix: keep comments in production and have a comment between v-if and v-else, encounter [vite:vue] v-else/v-else-if has no adjacent v-if or v-else-if. error

Why is test work?

Since the following code works in the test, the comments can be skipped.

if (__DEV__ && sibling && sibling.type === NodeTypes.COMMENT) {
context.removeNode(sibling)
comments.unshift(sibling)
continue
}

Steps to reproduce

  1. open the reproduction
  2. executed npm run build
  3. Error appears

@antfu
Copy link
Member

antfu commented Oct 12, 2022

Can you add some tests to it? Thanks

@Dunqing
Copy link
Contributor Author

Dunqing commented Oct 12, 2022

This test is the same as this issue, so I don't need to add the same cases for the test.

test('with comments', () => {
const { node } = parseWithIfTransform(`
<template v-if="ok">
<!--comment1-->
<div v-if="ok2">
<!--comment2-->
</div>
<!--comment3-->
<b v-else/>
<!--comment4-->
<p/>
</template>
`)

@LinusBorg LinusBorg added the ready to merge The PR is ready to be merged. label Oct 21, 2022
@yyx990803 yyx990803 closed this in dd3354c Nov 8, 2022
@yyx990803
Copy link
Member

Thanks for the PR. We still need a separate test case for this to better document the problem, and we should not introduce test-only logic to fix a case. See dd3354c

chrislone pushed a commit to chrislone/core that referenced this pull request Feb 4, 2023
zhangzhonghe pushed a commit to zhangzhonghe/core that referenced this pull request Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge The PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants