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

SyntaxError: Expected corresponding JSX closing tag #1691

Closed
lparedesl opened this issue Jan 12, 2022 · 15 comments · Fixed by #1696
Closed

SyntaxError: Expected corresponding JSX closing tag #1691

lparedesl opened this issue Jan 12, 2022 · 15 comments · Fixed by #1696
Labels
bug Something isn't working docz-utils Issues related to package docz-utils pending-release Issue fixed but not published to npm yet

Comments

@lparedesl
Copy link

Getting SyntaxError: Expected corresponding JSX closing tag error when using any component with children inside the Playground component.

Using components that self close, for example <MyComponent /> works fine.

Here is a sample with the error. Please click on Components > Alert on the left menu.

https://codesandbox.io/s/docz-example-xj522

Thanks

@panvourtsis
Copy link

i also tried other examples and all have the same error ....

@panvourtsis
Copy link

today i used the example below to add typescript support and all and it worked.

https://codesandbox.io/s/9pucf

If you delete the yarn.lock though and rerun the yarn install it will again break.

@tanbowensg
Copy link

same problem

@lparedesl
Copy link
Author

today i used the example below to add typescript support and all and it worked.

https://codesandbox.io/s/9pucf

If you delete the yarn.lock though and rerun the yarn install it will again break.

Thank you. I will try that

@bartlomiej-kochanowicz-landingi

any update?

@iandeherdt
Copy link

Same issue

@panvourtsis
Copy link

couldn't find what caused it.

@gmonte
Copy link

gmonte commented Jan 21, 2022

Same issue here.


<Playground>
  {() => {
    const foo = 'bar'
    return <div />
  }}
</Playground>

Screen Shot 2022-01-21 at 18 30 52


<Playground>
  {() => {
    const foo = 'bar'
    return <div></div>
  }}
</Playground>

Screen Shot 2022-01-21 at 18 30 33


<Playground>
  <div />
</Playground>

Screen Shot 2022-01-21 at 18 31 26


<Playground>
  <div></div>
</Playground>

Screen Shot 2022-01-21 at 18 31 13


I have the same behavior when I use with/without a function body. It is working just with auto-closing tags.

My env:
docz: 2.3.1
node: 16.13.1
yarn: 1.22.17
typescript: 4.4.2

@Elgeneinar
Copy link

Elgeneinar commented Jan 24, 2022

Same issue here. For me it seems to be cutting out the first ending tag, so

<Playground>
<div> </div>
<p> </p>
</Playground>

becomes

<Playground>
<div>
<p> </p>
</Playground>

@adbayb
Copy link
Contributor

adbayb commented Jan 28, 2022

After investigation, it seems that forcing (through yarn resolution or whatever) the downgrade of the package @babel/traverse to v7.16.7 (or @babel/core to v7.16.7) solves the issue.
The regression was introduced for versions greater or equal than v7.16.8.

It might be related to this issue babel/babel#14139 and this update babel/babel#14105

@nicolo-ribaudo
Copy link
Contributor

nicolo-ribaudo commented Jan 29, 2022

Hi! Babel maintainer here.

I'd be happy to help figure out if this is a Babel bug, or if it's a bug somewhere else that was hidden in this specific circumstance and unveiled by that Babel update.

Does docz use any custom Babel plugin to transform <Playground> tags? I only found https://github.com/doczjs/docz/tree/main/other-packages/babel-plugin-export-metadata but it looks unrelated.

@adbayb
Copy link
Contributor

adbayb commented Jan 31, 2022

Hello @nicolo-ribaudo 👋 ,
First of all, thanks for your time and involvement 🙏 .

I've mentioned the babel/babel#14105 since the issue starts to appear from @babel/traverse@v7.16.8 and above (same issue also with @babel/traverse@7.16.10).
From the changelog, I can see that the only change introduced since was babel/babel#14105.

Does docz use any custom Babel plugin to transform tags?

I'm not an active Docz maintainer but from what I see, It seems:

Capture d’écran 2022-01-31 à 09 11 38

Maybe the maintainers @pedronauck / @renatobenks can confirm.

@nicolo-ribaudo
Copy link
Contributor

After a very long debug session (it took a few hours because I was not familiar with the project 😬), I came to the conclusion that:


What is the fixed Babel bug exactly?

Babel has a few utility methods to control how the AST is traversed. One of them if path.stop(), whose goal is to enterly stop the current traversal. You can read its documentation in our semi-official plugins handbook.

However, in some circumstances it only prevented part of the remaining nodes from being traversed, rather than all of them. We fixed this bug, and now path.stop() behaves properly.


What is the docz bug?

docz has some code to remove the <Playground>...</Playground> tags, transforming

<Playground>
  <Alert>Some text here</Alert>
</Playground>

into

  <Alert>Some text here</Alert>

This is the utility function that should do that transformation:

export const removeTags = (code: string) => {
const open = codeFromNode(p => p.isJSXOpeningElement())
const close = codeFromNode(p => p.isJSXClosingElement())
return code.replace(open(code), '').replace(close(code), '')
}

For completeness, here is the codeFromNode function:

export const valueFromTraverse = (
condition: Condition,
predicate: Predicate = p => p
) => (code: string) => {
let value = ''
const ast = parser.parse(code, { plugins: ['jsx'] })
traverse(ast, {
enter(path: any): void {
if (condition(path)) {
value = predicate(path)
path.stop()
return
}
},
})
return value
}
export const codeFromNode = (condition: Condition) => (code: string) =>
valueFromTraverse(condition, p => code.slice(p.node.start, p.node.end))(code)

codeFromNode takes a "condition" function, and returns a function that, when given the input code, returns a portion of the code whose AST matches the condition.

const open = codeFromNode(p => p.isJSXOpeningElement()); open(code) returns the the code of something that matches p.isJSXOpeningElement().

In our example above, both <Playground> and <Alert> match that condition: which one does it return? If we look at the valueFromTraverse implementation, it:

  • parses the input source code
  • traverses it with @babel/traverse (Babel uses a DFS traversal, and when using the enter visitor it's in visits the node in pre-order).
  • when it finds a node that matches the condition, it stores it's value and calls path.stop() to stop the traversal.

Calling path.stop() as soon as something matches the condition means that it returns the first node that matches it. When looking for a JSXOpeningElement it thus returns <Playground>, while when looking for a JSXClosingElement it's </Alert>.

(It might help looking at the AST for my example, remembering that Babel traverses nodes in "source code order": when it comes to JSX, it first traverses the opening tag, then the children and then the closing tag.)

After getting <Playground> and </Alert>, removeTags removes them from the input code.

We were "lucky" before because path.stop() didn't work properly in that case, so it didn't stop the traversal when it found a closing tag but continued normally (reaching </Playground> at the end of the traversal).


My suggested fix

Rather than finding an opening and a closing tag separately, I suggest getting them at the same time. Also, the current .replace-based approach is quite fragile. For example (even before the @babel/traverse fix), this didn't work:

<Playground>
  This is followed by a comment! { /* </Playground> */ }
  <Alert>Some text here</Alert>
</Playground>

because by replacing <Playground> and </Playground> with an empty string you get:

  This is followed by a comment! { /*  */ }
  <Alert>Some text here</Alert>
</Playground>

I suggest to rewrite removeTags like this:

const getTagContentsRange = valueFromTraverse(
  p => p.isJSXElement(),
  ({ node }) => {
    if (!node.closingElement) {
      // if the JSX element doesn't have a closingElement, it's because it's self-closed
      // and thus does not have any content: <Playground />
      return [0, 0];
    }
    return [node.openingElement.end, node.closingElement.start];
  }
);

export const removeTags = (code: string) => { 
  const [start, end] = getTagContentsRange(code);
  return code.slice(start, end);
};

by doing so we get the start/end location of the contents of the first JSX tag, which should be safe.

@nicolo-ribaudo
Copy link
Contributor

I ended up creating a PR with the above fix - #1696

@renatobenks
Copy link
Member

great work @nicolo-ribaudo, we're going to merge and release it as soon as we can

@renatobenks renatobenks added bug Something isn't working docz-utils Issues related to package docz-utils pending-release Issue fixed but not published to npm yet labels Feb 7, 2022
nlopin added a commit to freenowtech/wave that referenced this issue Sep 7, 2022
The previous version contained a broken `Playground`. It has shown the error:

Expected corresponding JSX closing tag for <Accordion>

Issue describing the bug: doczjs/docz#1691
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working docz-utils Issues related to package docz-utils pending-release Issue fixed but not published to npm yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants