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

Blocked rescript@11 migration #467

Closed
DZakh opened this issue Oct 11, 2023 · 11 comments
Closed

Blocked rescript@11 migration #467

DZakh opened this issue Oct 11, 2023 · 11 comments

Comments

@DZakh
Copy link
Contributor

DZakh commented Oct 11, 2023

I've tried migrating the project at Carla to rescript@11 several times. I've fixed all the issues besides the problem with rescript-relay:

For context, we want to use uncurried: false since the codebase is quite big, and we want to gradually migrate to uncurried mode. Because of this, the rescript-relay V3 is not an option for us.

So we need to use rescript-relay V2 until we completely migrate to uncurried mode. But there's another problem. It uses _ prefixes for gql fields having restricted names. In rescript@10 the _s were removed in runtime, but after my PR 😅 I removed the fields mangling logic from rescript@11. This way, rescript-relay V2 stopped handling private keywords in rescript@11 properly, the fields in compiled js are now prefixed with _.

Later, I've added the PR to allow using @as with @obj so there's a migration path from fields mangling logic.

So, now I'd like to update rescript-relay V2 to use @as instead of _ for private keywords in the generated code. I've already taken a look in the source code and noticed a submodule written in Rust. If you tell me how to contribute to it, I'd like to give it a try. I really wish to start using rescript@11 in production.

@zth
Copy link
Owner

zth commented Oct 11, 2023

Sure! Check this repo out: https://github.com/zth/relay/tree/rescript-relay. It's the Relay compiler fork RescriptRelay uses. It's in the compiler/ dir.

Place yourself in the compiler/test-project-res dir and run cargo build && ../target/debug/relay. That'll compile the compiler and run it on the test project. You'll see any changes you make reflected there.

Adding @as() is hopefully fairly straight forward. There should be plenty of examples in the code base.

@mununki
Copy link

mununki commented Oct 17, 2023

Currently my project using "rescript": "v11-rc.4" and "rescript-relay": "^2.0.0" with uncurried: true. I have input type in graphQL schema:

input ShippingFeePolicyMutationInput {
  amount: Int!
  freeShippingThreshold: Int
  quantityStep: Int
  type: ShippingFeePolicyType!
}

The js output has _type instead of type now with this ReScript code.

Mutation.make_shippingFeePolicyMutationInput(~amount=0, ~_type=#FREE, ())
{
  amount: 0,
  _type: "FREE"
}

But, I can't figure it out why because playground looks fine to me. https://rescript-lang.org/try?version=v11.0.0-beta.4&code=AIGwlgbgpgUALgTwA5QARgHZIK5wPoDKAFmEkpgOYBiUUACgPbgDGCAsrgIZxgMYCSWXKgC8qAN4xUqTgFsG2DHABc6JQBopqAGYAnWsVLkMFACpF9AZyJMAJqoZIefADyY4APk3SAjtk5KYIgEcFBIDk68GG5KXlrAnJYAFABEiCgpAJSo6VB4qgDaAMQAwgDyAHIAIvym-JUAggAyeFQASgCiHagAPqhF7V29-RVlpngNdHRN-CUNAEJN3X1FdA38VcNFAIoAqg0VdaYAmnjzDQQdVQC6mgC+MDCgkGjADABGAFaoUAAeoboMJwQKhZJwANZ5axGSg0ehMMCsDhwbhRQQ4FSoJJaaQAPzkCiUqnc3mkqFxegMJDIlHMVhsIHsajgIgA-KS8X4AjxgqFwsy2RzyXhcoUcWSVuVqrV6hVmq1Oh1xdIVoMlWSJSMxhMpjM5ot1RqtmsNsqtnsDkdTudLlVxbdxYoglpsiIPGoMYRqcZqLRGCx2FxnAIhHBRKgUilHiAoGHZOGwZC8NCaSY4f7EYGUcH0bgkvj5IoWQAGdTC3IiAaKstJTKZIA

@mununki
Copy link

mununki commented Oct 17, 2023

The functions and type definition are extracted from the generated modules by rescript-relay.

@DZakh
Copy link
Contributor Author

DZakh commented Oct 17, 2023

It's the same issue I've described in the description.
That's because of the PR: rescript-lang/rescript#6354
Fix in the PR: zth/relay#15

@mununki
Copy link

mununki commented Oct 17, 2023

@DZakh Can you check the playground link I've added? I'm wondering why this output has type.
image

@DZakh
Copy link
Contributor Author

DZakh commented Oct 17, 2023

See rescript-lang/rescript#6354.
There used to be the logic of mangling object keys up till 11-rc.3

@DZakh
Copy link
Contributor Author

DZakh commented Oct 17, 2023

I think the change should be highlighted more in ReScript V11 release notes. It's a breaking change people should know about.

@mununki
Copy link

mununki commented Oct 17, 2023

Ah! I confused the setting of playground it was v11-beta.4 not rc.3 😓

@mununki
Copy link

mununki commented Oct 17, 2023

I opened an issue rescript-lang/rescript#6440, if the js output is generated by the return type of the external function, then _type issue here would be fixed, I guess.

@DZakh
Copy link
Contributor Author

DZakh commented Oct 17, 2023

Yes, but I think it'll be quite difficult to make it work. Also, if you noticed, in our case the type has a field called type_, while external has a field called _type.

@zth
Copy link
Owner

zth commented Oct 21, 2023

2.1.0 is out, and should fix this. Reopen if it does not.

@zth zth closed this as completed Oct 21, 2023
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