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

Add JSXNamespacedName to object of JSXMemberExpression #54

Closed
wants to merge 1 commit into from

Conversation

teppeis
Copy link

@teppeis teppeis commented Mar 13, 2016

Following JSX makes JSXNamespacedName as an object of JSXMemberExpression.

<n:v.p />

https://astexplorer.net/#/Vy64XFydgx

      {
        "type": "JSXElement",
        "start": 0,
        "end": 14,
        "openingElement": {
          "type": "JSXOpeningElement",
          "start": 0,
          "end": 14,
          "attributes": [],
          "name": {
            "type": "JSXMemberExpression",
            "start": 1,
            "end": 11,
            "object": {
              "type": "JSXMemberExpression",
              "start": 1,
              "end": 8,
              "object": {
                "type": "JSXNamespacedName",
                "start": 1,
                "end": 5,
                "namespace": {
                  "type": "JSXIdentifier",
                  "start": 1,
                  "end": 2,
                  "name": "n"
                },
                "name": {
                  "type": "JSXIdentifier",
                  "start": 3,
                  "end": 5,
                  "name": "v1"
                }
              },
              "property": {
                "type": "JSXIdentifier",
                "start": 6,
                "end": 8,
                "name": "v2"
              }
            },
            "property": {
              "type": "JSXIdentifier",
              "start": 9,
              "end": 11,
              "name": "v3"
            }
          },
          "selfClosing": true
        },
        "closingElement": null,
        "children": []
      }

@RReverser
Copy link
Contributor

AFAIK syntax spec doesn't allow such mix neither (intentionally), why do you think such AST would be valid?

@sebmck
Copy link

sebmck commented Mar 13, 2016

@RReverser See their link, it parses with acorn-jsx (and thus Babylon).

@teppeis
Copy link
Author

teppeis commented Mar 13, 2016

Yeah, it seems to be invalid for JSX spec:

JSXElementName :

  • JSXIdentifier
  • JSXNamespacedName
  • JSXMemberExpression

JSXNamespacedName :

  • JSXIdentifier : JSXIdentifier

JSXMemberExpression :

  • JSXIdentifier . JSXIdentifier
  • JSXMemberExpression . JSXIdentifier

So, is it acorn-jsx's bug?
Flow parser and TypeScript parser report syntax errors for it.

@RReverser
Copy link
Contributor

See #13 for previous discussion. So basically we decided to support syntactic form that (React) JSX doesn't use, but in a wider form to cover more use cases (like mentioned internationalization, but also for member expressions).

@teppeis
Copy link
Author

teppeis commented Mar 14, 2016

@RReverser Thanks. This should not be merged for consistency with the spec.

But the AST parsed by acorn-jsx is confusing.
IMO, it's better for acorn-jsx to throw "Invalid syntax" for <n:v.p />, or to throw "JSXNamespacedName is not supported" for <n:v /> if it only supports a subset of spec.

Related: acornjs/acorn-jsx#27

@teppeis teppeis closed this Mar 14, 2016
@teppeis teppeis deleted the patch-1 branch March 14, 2016 02:28
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.

None yet

3 participants