Skip to content

Regression after JSX3 removal #7084

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

Closed
cknitt opened this issue Oct 6, 2024 · 7 comments · Fixed by #7094
Closed

Regression after JSX3 removal #7084

cknitt opened this issue Oct 6, 2024 · 7 comments · Fixed by #7094
Assignees
Milestone

Comments

@cknitt
Copy link
Member

cknitt commented Oct 6, 2024

It seems something broke on master with the JSX3 cleanup PR #7072. I forgot to test against a real-world project before merging.

I have a large project with several yarn workspace packages and lots of dependencies.
Only the top-level rescript.json has

  "jsx": {
    "version": 4,
    "mode": "automatic"
  },

but not the other workspace packages or the dependencies.

It seems that the default of JSX 4 was previously inherited from the root rescript.json, but now that's not the case anymore.
So the project only builds if I add the above configuration to the rescript.json of all workspace packages and dependencies.

@mununki Could you have a look at this? And it would be great to have a test in build_tests to verify that the inheritance of the JSX setting works.

@cknitt cknitt added this to the v12 milestone Oct 6, 2024
@cristianoc
Copy link
Collaborator

It might default to off instead of inheriting in subprojects.
Btw should it ever be off?
I would leave it always on and only make the mode configurable.
Thoughts?

@cristianoc
Copy link
Collaborator

In any case the config mode should be inherited.

@mununki
Copy link
Member

mununki commented Oct 6, 2024

Previously, if a project had JSX-related configurations, the dependent packages would inherit those JSX settings even if they didn't have their own JSX configurations, right? Do you hope this will continue to be the default behavior in the future?

@cknitt
Copy link
Member Author

cknitt commented Oct 6, 2024

Do you hope this will continue to be the default behavior in the future?

Yes, the dependent packages should continue to inherit the JSX settings.

I would leave it always on and only make the mode configurable.
Thoughts?

Sounds good to me. Cannot really think of any major downside. It will be superfluous on non-React/-JSX projects, but it shouldn't do any harm there either.

@mununki
Copy link
Member

mununki commented Oct 7, 2024

  • JSX setting is inherited to the subprojects as default
  • The mode is configurable in the subprojects

Do I understand correctly?

@cknitt
Copy link
Member Author

cknitt commented Oct 8, 2024

@mununki We want to restore the previous behavior regarding inheritance that we had before the JSX3 removal PR.
I think it was like you described it here, but better double check.

@mununki
Copy link
Member

mununki commented Oct 10, 2024

@mununki We want to restore the previous behavior regarding inheritance that we had before the JSX3 removal PR. I think it was like you described it here, but better double check.

Got it.

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 a pull request may close this issue.

3 participants