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

Migrate to ReScript v11 #738

Merged
merged 30 commits into from
Dec 5, 2023

Conversation

aspeddro
Copy link
Collaborator

@aspeddro aspeddro commented Oct 29, 2023

  • JSX v4 and Uncurried mode enabled
  • Generate .bs.mjs instead .mjs, since we are using ESM

TODO:

  • Fix format playground

@aspeddro
Copy link
Collaborator Author

This PR allows us to move forward with document generation from docstrings compiler because @rescript/tools requires rescript v11

@aspeddro
Copy link
Collaborator Author

Ready for review

@ryyppy @zth

rescript.json Outdated Show resolved Hide resolved
src/SyntaxLookup.res Outdated Show resolved Hide resolved
@@ -57,11 +57,11 @@ module Link = {
~href: string,
~_as: string=?,
~prefetch: bool=?,
~replace: option<bool>=?,
Copy link
Member

Choose a reason for hiding this comment

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

Is this how it works in ReScript 11 now due to uncurried-by-default? Or was this wrong ever since?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Member

@ryyppy ryyppy left a comment

Choose a reason for hiding this comment

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

Okay I noticed we actually already vendored the encoding / decoding lib. I still feel like the extra manual Js.Json decoding is a little unweildy and annoying to maintain... any ways to keep the original logic somehow?

@aspeddro
Copy link
Collaborator Author

aspeddro commented Nov 2, 2023

I removed Json interface files because they are reporting signature errors.

rescript-lang/rescript#6413

@ryyppy
Copy link
Member

ryyppy commented Nov 3, 2023

I guess this is kinda related to #739 \cc @cknitt ?

@aspeddro
Copy link
Collaborator Author

@ryyppy I merged #739 in this PR.

But I got some errors which I think are related to rescript-lang/rescript#6413

When I create a Json_encode.resi

  We've found a bug for you!
  /home/pedro/Desktop/projects/rescript-lang.org/src/vendor/Json_encode.res:35:5-9

  33 │
  34 │ external jsonArray: array<Js.Json.t> => Js.Json.t = "%identity"
  35 │ let array = (encode, l) => jsonArray(Array.map(encode, l))
  36 │ let list = (encode, x) =>
  37 │   switch x {

  The implementation /home/pedro/Desktop/projects/rescript-lang.org/src/vendor/Json_encode.res
  does not match the interface src/vendor/Json_encode.cmi:
  Values do not match:
    let array: ('a => Js.Json.t, array<'a>) => Js.Json.t
  is not included in
    let array: ('a => Js.Json.t, array<'a>) => Js.Json.t
  /home/pedro/Desktop/projects/rescript-lang.org/src/vendor/Json_encode.resi:88:1-92:52:
    Expected declaration
  /home/pedro/Desktop/projects/rescript-lang.org/src/vendor/Json_encode.res:35:5-9:
    Actual declaration

FAILED: cannot make progress due to previous errors.

@ryyppy
Copy link
Member

ryyppy commented Nov 15, 2023

^ @cknitt any idea?

@cristianoc
Copy link
Contributor

@ryyppy I merged #739 in this PR.

But I got some errors which I think are related to rescript-lang/rescript-compiler#6413

When I create a Json_encode.resi

  We've found a bug for you!
  /home/pedro/Desktop/projects/rescript-lang.org/src/vendor/Json_encode.res:35:5-9

  33 │
  34 │ external jsonArray: array<Js.Json.t> => Js.Json.t = "%identity"
  35 │ let array = (encode, l) => jsonArray(Array.map(encode, l))
  36 │ let list = (encode, x) =>
  37 │   switch x {

  The implementation /home/pedro/Desktop/projects/rescript-lang.org/src/vendor/Json_encode.res
  does not match the interface src/vendor/Json_encode.cmi:
  Values do not match:
    let array: ('a => Js.Json.t, array<'a>) => Js.Json.t
  is not included in
    let array: ('a => Js.Json.t, array<'a>) => Js.Json.t
  /home/pedro/Desktop/projects/rescript-lang.org/src/vendor/Json_encode.resi:88:1-92:52:
    Expected declaration
  /home/pedro/Desktop/projects/rescript-lang.org/src/vendor/Json_encode.res:35:5-9:
    Actual declaration

FAILED: cannot make progress due to previous errors.

Need:

let array = (encode, l) => jsonArray(Array.map(x => encode(x), l))

The change is x => encode(x) which calls encode as an uncurried function.

@aspeddro
Copy link
Collaborator Author

Thanks Cristiano!!

Added .resi files for Json

@aspeddro
Copy link
Collaborator Author

Getting an warning 45

https://github.com/aspeddro/rescript-lang.org/blob/d6624a81fba01a1c852d4e92a269a46c2e75173b/src/layouts/DocsLayout.res#L200-L211

SidebarLayout.Toc module.


  Warning number 45
  /home/pedro/Desktop/projects/rescript-lang.org/src/layouts/DocsLayout.res:203:9-209:24

  201 ┆   open Belt.Option
  202 ┆   Js.Dict.get(Content.tocData, route)->map(data => {
  203 ┆     open SidebarLayout.Toc
  204 ┆     let title = data["title"]
    . ┆ ...
  208 ┆     })
  209 ┆     {title, entries}
  210 ┆   })
  211 ┆ }

  this open statement shadows the label title (which is later used)

@fhammerschmidt fhammerschmidt merged commit b73bac8 into rescript-lang:master Dec 5, 2023
1 check passed
Copy link
Member

@ryyppy ryyppy left a comment

Choose a reason for hiding this comment

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

LGTM

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.

5 participants