Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: add jsx support#391

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
erquhart wants to merge2 commits intoestools:master
base:master
Choose a base branch
Loading
fromerquhart:feat/jsx
Draft

Conversation

@erquhart
Copy link

Adds JSX support.

Notes

  • Changed the general test suite to use the esprima dependency rather than 1.0.0-dev
  • Upgraded esprima from 3.1.3 to 4.0.1 for improved JSX support in tests
  • Esprima upgrade is causing tests to fail for two reasons:
    • Esprima outputs string literals with double quotes in 1.0.0, but switched to single quotes in 1.1.0 (source)
    • Esprima no longer represents the presence of a leading 0 in a decimal, so instead of "0.14" it's ".14", both forvalue andraw
  • Besides the above mentioned issues, tests are passing
  • JSX SpreadChild and Fragments are not supported in this PR, as support for them is not quite widespread (esprima supports neither, I'd consider that to be a decent indicator)

Todo

  • determine if any cases merit throwing an error
  • format output appropriately for compact/non-compact
  • precedence/flag usage review by a maintainer for all uses ofgenerateExpression()

Closes#333.

jen-Ya reacted with thumbs up emoji
Copy link
Contributor

@papandreoupapandreou left a comment

Choose a reason for hiding this comment

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

I'm no JSX expert, I just wanted to get serialization support in assetgraph. Thanks a lot for picking this up!

I triednpm linking escodegen in with this change, and it seems to do the job! 👍

Would be nice to get support forJSXFragment while you're at it. It's not supported by esprima yet, but you can test that with acorn or espree, for example.

jen-Ya reacted with thumbs up emoji
if(expr.value.type==='JSXExpressionContainer'){
result.push(this.generateExpression(expr.value.expression,precedence,flags));
returnresult;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You also need to handleexpr.value.type === 'JSXElement' here.

>require('escodegen').generate(require('esprima').parse('(<Foo bar=<Quux/>/>)',{jsx:true}));'<Foo undefined />;'

AndJSXFragment as well, if you go for that:https://github.com/facebook/jsx/blob/master/AST.md#jsx-attributes

Copy link
Author

Choose a reason for hiding this comment

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

Totally forgot about boolean attributes, good catch 👍

Missed it, used to be `[space, '/>]`, forgot to drop the array.Co-Authored-By: erquhart <shawn@erquh.art>
@erquhart
Copy link
Author

Haven't forgotten about this, will circle back soon.

papandreou, hezyin, jen-Ya, and zouxuoz reacted with thumbs up emoji

@zouxuoz
Copy link

@erquhart how I can help you to finish this update?

@erquhart
Copy link
Author

Just a matter of when, haven't circled back yet. An old fork (https://github.com/ng-vu/escodegen-jsx) that introduced (incompatible) jsx support had some things that felt necessary, so the plan has been to pull in the more thorough bits from there. If you want to take this across the finish line, by all means please do!

@sag1v
Copy link

Is JSX going to be supported or is there any other way to do it?

@smol-honk
Copy link

What if we added thishttps://github.com/wallabyjs/escodegen?

erquhart and sag1v reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

1 more reviewer

@papandreoupapandreoupapandreou left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

this[type] is not a function

5 participants

@erquhart@zouxuoz@sag1v@smol-honk@papandreou

[8]ページ先頭

©2009-2025 Movatter.jp