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

fix @deriving clash in rescript 9 #174

Merged
merged 1 commit into from Mar 30, 2021
Merged

Conversation

@vercel
Copy link

vercel bot commented Mar 30, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/davesnx/styled-ppx/B3LscnXtN1nsQwf7T8Ju7Dd8Eh3J
✅ Preview: https://styled-ppx-git-fork-jfrolich-fix-rescript-9-davesnx.vercel.app

@AlexMoutonNoble
Copy link

AlexMoutonNoble commented Feb 13, 2023

Hi @davesnx
Has this made it into an proper release of styled-ppx or should i be pulling from git directly?

I think I am seeing this in 0.33.4?

Thanks
Alex

@davesnx
Copy link
Owner

davesnx commented Feb 14, 2023

Hi @AlexMoutonNoble, this was merged a while ago. It shouldn't happen.

Let me try to reproduce and meanwhile if you have a repo with the issue please share it in an issue please!

Thanks for jumping into the PR

@AlexMoutonNoble
Copy link

AlexMoutonNoble commented Feb 14, 2023

Is there some cache of installed ocaml code that I may have old/conflicting versions installed in? Taking our web directory into a new git repo and building seems to succeed?

We're also using graphql-ppx fwiw.

@davesnx
Copy link
Owner

davesnx commented Feb 14, 2023

Can you make sure to delete node_modules or at least styled-ppx folder and try again?

@AlexMoutonNoble
Copy link

reset node_modules and still doing it. hmm.

@AlexMoutonNoble
Copy link

@davesnx does this one work for you? https://github.com/noble-ai/styled-ppx-clash

@davesnx
Copy link
Owner

davesnx commented Feb 15, 2023

Indeed fails, but the issue comes from the @deriving not much to do with styled-ppx (AFAIK)

FAILED: src/navi.ast

  We've found a bug for you!
  /Users/davesnx/Code/playground/styled-ppx-clash/src/navi.res:1:11-18

  1 │ @deriving(abstract)
  2 │ type urlDescriptorPartial = {
  3 │   @optional pathname: string,

  Ppxlib.Deriving: 'abstract' is not a supported type deriving generator

In your demo, there are some dependencies missing: rescript-react, bs-css and bs-css-emotion.

If you can try with https://github.com/davesnx/try-styled-ppx

@davesnx
Copy link
Owner

davesnx commented Feb 15, 2023

Btw, feel free to jump on Discord and have it sorted: https://discord.gg/reasonml

@AlexMoutonNoble
Copy link

AlexMoutonNoble commented Feb 15, 2023

The thing is, if i remove the styled-ppx, the example code compiles fine.
Is this a rescript 10 thing?
Will join the discord...thanks

A

@davesnx
Copy link
Owner

davesnx commented Feb 15, 2023

Discovered the issue. Pushed a new version with a fix (0.34.0). I forgot to update linkall in the binary as well (was pushing styled-ppx + ppxlib) without knowing it.

As a side note, in ReScript the syntax for payload to work with styled-ppx is using " not `: #308

Probably worth fixing that issue as well ^^

@AlexMoutonNoble
Copy link

Rad thanks David. I dont see 0.34 pushed up yet but ill keep an eye out.

@davesnx
Copy link
Owner

davesnx commented Feb 18, 2023

It's published (maybe it took a little longer after my comment?): https://www.npmjs.com/package/@davesnx/styled-ppx/v/0.34.0

Nevermind, let me know if you run into any issues we battle-tested at ahrefs.com but we don't use it in ReScript and some problems might appear, happy to fix them fast.

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.

None yet

3 participants