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

Support JSX Element as JSX Attribute Value #7410

Closed
JamesHenry opened this issue Mar 6, 2016 · 8 comments · Fixed by #47994
Closed

Support JSX Element as JSX Attribute Value #7410

JamesHenry opened this issue Mar 6, 2016 · 8 comments · Fixed by #47994
Labels
Bug A bug in TypeScript Domain: JSX/TSX Relates to the JSX parser and emitter Effort: Casual Good issue if you're already used to contributing to the codebase. Harder than "good first issue". Help Wanted You can do this
Milestone

Comments

@JamesHenry
Copy link
Contributor

TypeScript Version:

1.8.2

Background:

I have been working on trying to get https://github.com/eslint/typescript-eslint-parser, a TypeScript parser plugin for ESLint, off the ground, and the job this weekend has been to begin adding JSX support.

ESLint uses espree and so the aim of the project is to convert the output of the tsc to an AST which espree expects. We already have a solid suite of JSX tests to develop against (taken from the espree project itself), but I have come up against a tsc error in one of them so far.

My issue is that it seems currently the tsc does not accept this "embedded-tags" JSX syntax:

Code

<LeftRight left=<a /> right=<b>monkeys /> gorillas</b> />;

Expected behavior:
Please note the AST produced by espree (see in particular the tokens array): http://astexplorer.net/#/9WAjizSvbC

Actual behavior:
...compared to the one produced by the tsc (note the issues found in parseDiagnostics):
http://astexplorer.net/#/YehKzLJYMW

@DanielRosenwasser DanielRosenwasser added the Needs More Info The issue still hasn't been fully clarified label Mar 6, 2016
@DanielRosenwasser
Copy link
Member

That doesn't look like valid JSX to begin with. Can you point out where on this page it's specified otherwise?

@DanielRosenwasser DanielRosenwasser added the Domain: JSX/TSX Relates to the JSX parser and emitter label Mar 6, 2016
@JamesHenry
Copy link
Contributor Author

As far as I can tell from that spec the supported values for a tag attribute are:

JSXAttributeValue:

  • " JSXDoubleStringCharactersopt "
  • ' JSXSingleStringCharactersopt '
  • { AssignmentExpression }
  • JSXElement

Both <a /> and <b>monkeys /> gorillas</b> should be valid JSXElements:

E.g. via babel:

<a />: https://babeljs.io/repl/#?evaluate=true&presets=es2015%2Creact&experimental=false&loose=false&spec=false&code=%3Ca%20%2F%3E

<b>monkeys /> gorillas</b>: https://babeljs.io/repl/#?evaluate=true&presets=es2015%2Creact&experimental=false&loose=false&spec=false&code=%3Cb%3Emonkeys%20%2F%3E%20gorillas%3C%2Fb%3E

Therefore, based on that spec, those two should also be valid JSXAttributeValues.

Interestingly, babel also does not seem to like the full LeftRight element:
https://babeljs.io/repl/#?evaluate=true&presets=es2015%2Creact&experimental=false&loose=false&spec=false&code=%3CLeftRight%20left%3D%3Ca%20%2F%3E%20right%3D%3Cb%3Emonkeys%20%2F%3E%20gorillas%3C%2Fb%3E%20%2F%3E%3B

But, hey, it may well be a bug in babel :)

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Help Wanted You can do this and removed Needs More Info The issue still hasn't been fully clarified labels Mar 7, 2016
@RyanCavanaugh RyanCavanaugh self-assigned this Mar 7, 2016
@RyanCavanaugh
Copy link
Member

Wow, I've literally never seen this.

Should be easy enough to parse with a one-token lookahead. Without real-world use this is going to be lower-priority but we'd take a PR at any time.

@RyanCavanaugh RyanCavanaugh added this to the TypeScript 2.1 milestone Mar 7, 2016
@JamesHenry
Copy link
Contributor Author

I have not contributed to the project before, but I might be able to find some time in the coming weeks. Do you have any documentation other than https://github.com/Microsoft/TypeScript/blob/master/CONTRIBUTING.md for getting set up with developing fixes/features?

The main thing I want to know is how the tests are put together and how I would run particular tests, rather than all 80,000+ cases?

@RyanCavanaugh
Copy link
Member

jake runtests tests=someFilenameMatch

For adding tests, it's probably easiest to look at past PRs for examples.

One trick to know is jake baseline-accept[soft] which will copy over only baselines for the subset of tests you ran, rather than nuking the entire reference folder.

@JamesHenry
Copy link
Contributor Author

Great, thanks!

@RyanCavanaugh
Copy link
Member

FWIW this was added to the JSX spec between March 4th and March 15th of 2015. So don't rely on the "(c) 2014" footer as a "last-changed" date 😄

Mar 4: https://web.archive.org/web/20150304180404/http://facebook.github.io/jsx
Mar 15: https://web.archive.org/web/20150315055942/http://facebook.github.io/jsx/

@RyanCavanaugh
Copy link
Member

Logged facebook/jsx#53

@mhegazy mhegazy modified the milestones: Future, TypeScript 2.1 Sep 29, 2016
@RyanCavanaugh RyanCavanaugh removed their assignment Aug 15, 2017
@RyanCavanaugh RyanCavanaugh changed the title JSX embedded tags syntax not supported Support JSX Element as JSX Attribute Value Dec 15, 2021
@RyanCavanaugh RyanCavanaugh added the Effort: Casual Good issue if you're already used to contributing to the codebase. Harder than "good first issue". label Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: JSX/TSX Relates to the JSX parser and emitter Effort: Casual Good issue if you're already used to contributing to the codebase. Harder than "good first issue". Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants