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

[Feature request] JSX version 4 #327

Closed
whitchapman opened this issue Mar 3, 2023 · 17 comments
Closed

[Feature request] JSX version 4 #327

whitchapman opened this issue Mar 3, 2023 · 17 comments

Comments

@whitchapman
Copy link

I use this ppx with Rescript, and the error message I get when attempting to use version 4 is:

  The record field children can't be found.

  If it's defined in another module or file, bring it into scope by:
  - Prefixing it with said module name: TheModule.children
  - Or specifying its type:
  let theValue: TheModule.theType = {children: VALUE}

Thanks!

@ashton
Copy link

ashton commented Mar 21, 2023

Is there anything we can help with that? I don't have knowledge creating PPX but if there is some guide of manual work that is needed I can help with.

@davesnx
Copy link
Owner

davesnx commented Mar 21, 2023

There are a few doubts from my side, help resolving those might make the feature available very soon (depending on the answers obviously!).

  • Would you expect styled-ppx to transform to JSX4 for all files? Would you expect to read bs-config jsx field or a ppx flag styled-ppx/ppx --jsx4?
  • What happens when JSX4 isn't enabled? Explode?
  • Do we want to support the v3-compatible mode?
  • Since we generate React components, do we want to support the mode as well?
  • What happens when JSX4 is enabled in .re files? Should automatically use JSX3 for these cases or crash?

@davesnx
Copy link
Owner

davesnx commented Mar 21, 2023

I posted an issue for the ReScript team about some technical issue with the generation of props, If anyone have a good idea, tune in. Let's see how this goes, but meanwhile would love to know some opinions on the above.

The post is https://forum.rescript-lang.org/t/ann-new-nightly-release-of-styled-ppx-working-with-jsx4/4331/2,

@davesnx
Copy link
Owner

davesnx commented Mar 21, 2023

I published a new alpha version that has dynamic components broken, the rest should work as expected.

npm install @davesnx/[email protected]

@davesnx
Copy link
Owner

davesnx commented Mar 23, 2023

Pushed a new version with most of the fixes, should be a little stable now.

npm install @davesnx/styled-ppx@nightly

A few things to take into account:

  • It reads the bs-config.json and applies the JSX4 transformation to all styled components.
  • It doesn't support reading the decorator @@jsxVersion.
  • It only does the "classic" method.

@davesnx davesnx changed the title [Feature request] Jsx version 4 support [Feature request] JSX version 4 Mar 25, 2023
@davesnx
Copy link
Owner

davesnx commented Mar 31, 2023

I merged the PR with JSX4 support (v0.35), it's currently in beta and append here any issue you find with it.

@whitchapman
Copy link
Author

whitchapman commented Mar 31, 2023

Thanks for doing this. Unfortunately, I still get the exact same compile error when setting jsx to version 4 using version 0.35 of the ppx. It's possible that I am missing something basic. EDITED: removed superfluous comments

@davesnx
Copy link
Owner

davesnx commented Mar 31, 2023

Ok, miss-typed the version and forgot to publish it. It's 0.36. Can you try updating?

@whitchapman
Copy link
Author

Ok, miss-typed the version and forgot to publish it. It's 0.36. Can you try updating?

Yes, that did it. Outstanding!

@ashton
Copy link

ashton commented Apr 12, 2023

Hello @davesnx! First let me thank you for being so quick and prestative! I don't know if I'm doing something wrong, but when I do:

module CardBody = %styled.div(`
  display: flex;
  justify-content: space-between;
`)

@react.component
let make = () => {
  <CardBody className="card-body">
     {"... many children here"}
  </CardBody>
}

Using the option at bsconfig.json:

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

It should render the CardBody component with the generated class name for the style that I provided using the %styled extension, and also concatenate it with the class name that I provide in the className argument right?

That is not what is happening:
image
image

I'm using the 0.36.2 version of styled-ppx

@davesnx
Copy link
Owner

davesnx commented Apr 13, 2023

Right, it seems like className isn't appended. Will have a look.

Thanks for trying it <3

@ashton
Copy link

ashton commented Apr 27, 2023

Heeey, any news?

@davesnx
Copy link
Owner

davesnx commented May 5, 2023

Pushed 0.38.1 fixing the className issue. Thanks for the testing!

@davesnx
Copy link
Owner

davesnx commented May 5, 2023

The missing piece on JSX4 is the new JSX transformation.

@davesnx
Copy link
Owner

davesnx commented Nov 8, 2023

Hey @ashton @whitchapman

Do you have any OSS repository with ReScript that is using v11?

@ashton
Copy link

ashton commented Dec 10, 2023

Do you have any OSS repository with ReScript that is using v11?

Hey, I don't think v11 is out yet, but as soon as it gets released I'll update my projects with it

@davesnx
Copy link
Owner

davesnx commented Jan 30, 2024

Finishing the v11 support on #415

JSX4 should work nice

@davesnx davesnx closed this as completed Jan 30, 2024
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

No branches or pull requests

3 participants