This repository has been archived by the owner on Jun 15, 2023. It is now read-only.
Clean up Js.t object parsing, printing and converting #291
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes rescript-lang#277 Fixes rescript-lang#263 Now that rescript-lang/rescript#4967 has landed: - Parse `{"foo": int}` as ocaml `{. foo: int}`. Previously it parsed into ocaml `{. foo: int} Js.t` - Remove a tiny printing optimizations for `Js.t`. - For React's PPX 3, generate objects directly instead of `Js.t` objects. cc @rickyvetter @ryyppy for ppx4. - The re->res converter automatically removes the `Js.t` part. - Said converter has a bug (rescript-lang#263) that converts `Js.t({..}) as 'a` into `{..} as 'a` from naturally forgetting to special-case that path. Now this bug is conveniently ~~made into a feature~~ obsolete.
IwanKaramazow
approved these changes
Feb 27, 2021
src/res_printer.ml
Outdated
|
|
||
| (* object printings *) | ||
| | Ptyp_object (fields, openFlag) -> | ||
| printBsObjectSugar ~inline:false fields openFlag cmtTbl |
There was a problem hiding this comment.
Should we improve the name of this function?
There was a problem hiding this comment.
Was thinking of unifying this with record printing, which we can discuss later. I'll do this now.
| @@ -327,7 +327,8 @@ external test: ( | |||
| foo, | |||
| bar, | |||
| baz, | |||
| ) => @attr {.. | |||
| ) => @attr | |||
There was a problem hiding this comment.
The line break is shady.
There was a problem hiding this comment.
I think before that, it went into a branch where it saw an attribute right before a type constructor ( Edit: wait no that's not true, master printed any constructor on a new lineJs.t), so formatted that Js.t on the same line as the attribute, which then triggered the type constructor + object <{ hugging branch.
But I've already discovered an existing error! @foo Js.t<{"bar": baz}> printing removes the @foo.
|
Let's maybe also add a test for |
The removal of this printer logic fixes a bug where `type a = @b Js.t<{"a": b}>`, when printed, loses the attribute
chenglou
added a commit
to chenglou/syntax
that referenced
this pull request
Mar 8, 2021
anmonteiro
added a commit
to melange-re/melange
that referenced
this pull request
Mar 25, 2021
anmonteiro
added a commit
to melange-re/melange
that referenced
this pull request
Mar 26, 2021
anmonteiro
added a commit
to melange-re/melange
that referenced
this pull request
Oct 30, 2021
anmonteiro
added a commit
to melange-re/melange
that referenced
this pull request
Oct 30, 2021
* Upgrade syntax repo to d9c44bb22 rescript-lang/syntax@d9c44bb * napkin: revert the rest of upstream PR#291 (#61) rescript-lang/syntax#291
anmonteiro
added a commit
to melange-re/melange
that referenced
this pull request
Feb 18, 2022
anmonteiro
added a commit
to melange-re/melange
that referenced
this pull request
Feb 18, 2022
…69c73 (#241) * Revert "Upgrade (ReScript) syntax repo to d9c44bb22 (#214)" This reverts commit 7701ced. * upgrade to b06dfa8fc6da202318bedaed66fb45a7fab69c73 * napkin: revert the rest of upstream PR#291 (#61) rescript-lang/syntax#291 * Ignore some test files for now while we can't upgrade rescript
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #277
Fixes #263
Now that rescript-lang/rescript#4967 has landed:
{"foo": int}as ocaml{. foo: int}. Previously it parsed into ocaml{. foo: int} Js.tJs.t.Js.tobjects. cc @rickyvetter @ryyppy for ppx4.Js.tpart.Js.t({..}) as 'ainto{..} as 'afrom naturally forgetting to special-case that path. Now this bug is convenientlymade into a featureobsolete.