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(ssr): v-slot stop empty attribute producing syntax error #3326

Closed
wants to merge 39 commits into from

Conversation

tjk
Copy link
Contributor

@tjk tjk commented Mar 1, 2021

template(#footer="")

currently produces an SSR render function with a syntax error (no first param):

default: _withCtx((, _push, _parent, _scopeId) => {

The fix is probably not right but don't know internals enough to know where it should be fixed or how this should be handled better. As I left a comment, if this should error instead of just work... it probably still requires changes because currently you just end up with a stack trace for compiled source code which you cannot see, for example:

SyntaxError: Unexpected token (108:22)
    at Object.pp$4.raise (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:37624:13)
    at Object.pp.unexpected (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:35315:8)
    at Object.pp$3.parseExprAtom (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:37022:10)
    at Object.pp$3.parseExprSubscripts (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:36825:19)
    at Object.pp$3.parseMaybeUnary (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:36802:17)
    at Object.parseMaybeUnary (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:45345:29)
    at Object.pp$3.parseExprOps (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:36737:19)
    at Object.pp$3.parseMaybeConditional (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:36720:19)
    at Object.pp$3.parseMaybeAssign (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:36688:19)
    at Object.pp$3.parseParenAndDistinguishExpression (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:37118:28)
    at Object.pp$3.parseExprAtom (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:36982:41)
    at Object.pp$3.parseExprSubscripts (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:36825:19)
    at Object.pp$3.parseMaybeUnary (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:36802:17)
    at Object.parseMaybeUnary (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:45345:29)
    at Object.pp$3.parseExprOps (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:36737:19)
    at Object.pp$3.parseMaybeConditional (/Users/tj/src/github.com/vitejs/vite/packages/vite/dist/node/chunks/dep-ed571b05.js:36720:19) {
  pos: 5439,
  loc: Position { line: 108, column: 22 },
  raisedAt: 5440
}

@tjk
Copy link
Contributor Author

tjk commented Mar 1, 2021

Hm, sort of wish I could call genNode(props) and just see if it returns empty... (it wouldn't make sense to delay the defaulting to _ into codegen internals I imagine)

Ooh, there is stringifyExpression but not exported.

@LinusBorg LinusBorg added 🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. scope: compiler scope: slots labels Mar 9, 2021
@@ -90,6 +90,11 @@ describe('ssr: components', () => {
`)
})

test('empty attribute should not produce syntax error', () => {
// previously this would produce syntax error `default: _withCtx((, _push, ...)`
expect(compile(`<foo v-slot="">foo</foo>`).code).not.toMatch(`(,`)

This comment was marked as outdated.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I didn't see the comment above

@HcySunYang HcySunYang requested a review from LinusBorg March 12, 2021 02:59
@tjk
Copy link
Contributor Author

tjk commented Apr 7, 2021

Bump? Also is there something that could be done so that acorn produces better output for the syntax error since it doesn't map to visible source?

webfansplz and others added 24 commits October 3, 2022 16:37
…6812)

Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>
… types (fix: vuejs#2855) (vuejs#5458)

Co-authored-by: Carlos Rodrigues <carlos@hypermob.co.uk>
yyx990803 added a commit that referenced this pull request Oct 26, 2022
@yyx990803
Copy link
Member

Sorry for letting this hang for so long - somehow GitHub seems to fail to recognize updates in your branch even after I resolved the conflicts, so I had to manually land the changes in 09bb3e9. Thanks again!

@yyx990803 yyx990803 closed this Oct 26, 2022
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
🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. scope: compiler scope: slots scope: ssr
Projects
None yet
Development

Successfully merging this pull request may close these issues.