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.

Sign up for GitHub

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

Jump to bottom

JS: Improve JSX lexing (support fragment syntax and embedded comments) #5765

Merged
merged 2 commits into from
Apr 7, 2023
Merged

JS: Improve JSX lexing (support fragment syntax and embedded comments) #5765

merged 2 commits into from
Apr 7, 2023

Conversation

matthiasblaesing
Copy link
Contributor

There are two issues in the JSX lexers:

  • embedded comments in assignment blocks are not correctly handled
  • fragment syntax (tag without name <></> was not supported

To visualize (on the left is the before picture, on the right is the after picture):

image

Closes: #5149
Closes: #4832

@matthiasblaesing matthiasblaesing added JavaScript [ci] enable web job and extra JavaScript tests (webcommon/javascript2.editor) ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Apr 2, 2023
@matthiasblaesing matthiasblaesing added this to the NB18 milestone Apr 2, 2023
@matthiasblaesing matthiasblaesing mentioned this pull request Apr 6, 2023
Merged
junichi11
junichi11 approved these changes Apr 7, 2023
View reviewed changes
Copy link
Member

@junichi11 junichi11 left a comment

Choose a reason for hiding this comment

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

I've verified that there is no error with the example code.
nb-gh-5765-1
nb-gh-5765-2

@matthiasblaesing matthiasblaesing removed the ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) label Apr 7, 2023
@matthiasblaesing
Copy link
Contributor Author

@junichi11 thank you for the check. I'm updating this PR in place with the fix for the formatting and will merge once it is green.

I removed the ci:dev-build label as a new build is not necessary (unittests will be run).

@matthiasblaesing matthiasblaesing merged commit c7847c6 into apache:master Apr 7, 2023
@matthiasblaesing matthiasblaesing deleted the js_improvements3 branch April 7, 2023 10:15
@moeenio
Copy link

moeenio commented Apr 7, 2023

Thank you very much!

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

@junichi11 junichi11 junichi11 approved these changes

Assignees
No one assigned
Labels
JavaScript [ci] enable web job and extra JavaScript tests (webcommon/javascript2.editor)
Projects
None yet
Milestone
NB18
3 participants
@matthiasblaesing @moeenio @junichi11