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 support for JSX #480

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add support for JSX #480

wants to merge 6 commits into from

Conversation

wooorm
Copy link

@wooorm wooorm commented Mar 7, 2021

  • Add new JSX export, similar to GENERATOR, with handlers for JSX nodes
  • Add unit tests for JSX nodes
  • Add JSX example

  • I’m a fan of small unit tests over checking whole documents, so I tested the new features with those
  • Types are not tested, and I’m not a TS user, so I didn’t add types for JSX. For reference, if you’re interested, here is an example of how to test types. You could also opt to use JSDoc for typescript if the source is JS, which I quite like, like so. Some complexity with types though is that newer ESTree proposal aren’t typed yet, such as bigint (and I’m not sure where JSX is typed)

* Add new `JSX` export, similar to `GENERATOR`, with handlers for
  JSX nodes
* Add unit tests for JSX nodes
* Add JSX example
@codecov
Copy link

codecov bot commented Mar 7, 2021

Codecov Report

Merging #480 (8b7f1dd) into master (2dfdef1) will increase coverage by 0.77%.
The diff coverage is 99.26%.

❗ Current head 8b7f1dd differs from pull request most recent head 9f1bbaf. Consider uploading reports for the commit 9f1bbaf to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #480      +/-   ##
==========================================
+ Coverage   94.97%   95.74%   +0.77%     
==========================================
  Files           1        1              
  Lines        1134     1270     +136     
==========================================
+ Hits         1077     1216     +139     
+ Misses         57       54       -3     
Impacted Files Coverage Δ
src/astring.js 95.74% <99.26%> (+0.77%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2dfdef1...9f1bbaf. Read the comment docs.

ChristianMurphy
ChristianMurphy previously approved these changes Mar 7, 2021
@davidbonnet
Copy link
Owner

davidbonnet commented Apr 5, 2021

Thanks @wooorm for sending this, it looks great! Adding inline comments about what a node does is also a great idea.
The reason behind template-based tests is to avoid malformed AST nodes and to ensure that sourcemaps match.

I'll have more time next week-end to apply the remaining changes:

  • Add template tests
  • Add TypeScript types
  • Add generateJsx helper
  • Update demo with JSX support

Edit: not this one but the next one.

@wooorm
Copy link
Author

wooorm commented Apr 5, 2021

here’s a PR somewhere else that types (using JSDoc comments, which I like a lot) these same functions: wooorm/xdm#24, by @ChristianMurphy

@davidbonnet davidbonnet self-assigned this Jun 2, 2021
@davidbonnet
Copy link
Owner

Just a quick note to mention that I haven't forgotten this PR.

@wooorm
Copy link
Author

wooorm commented Jun 2, 2021

Let me now if there's anything I could do to unblock it!

@Adam-Collier
Copy link

Hi guys, I'm looking to do exactly this! Is there any update on when this will be merged, or can I help in any way?

@wooorm
Copy link
Author

wooorm commented Jan 25, 2022

Friendly ping!

@wooorm
Copy link
Author

wooorm commented Oct 12, 2022

@davidbonnet Friendly ping! Anything I can do?

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.

4 participants