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

feat: add support for @rescript/react, urql 2, and bs-platform 9 #254

Merged
merged 3 commits into from
Mar 21, 2021

Conversation

parkerziegler
Copy link
Contributor

@parkerziegler parkerziegler commented Mar 7, 2021

Fix #253

This PR begins the process of migrating us to a whole slate of new dependencies:

  • @rescript/react — this is where the ReScript core team will be maintaining React moving forward.
  • urql@2.0.1 — this needs no explanation 😅
  • bs-platform@9.0.1 — this just picks up the new compiler version and, fortunately, we have no breaking code!

I also removed the pollInterval option per the urql@2.0.0 CHANGELOG and updated a few internals to support this.

The next step in this migration is to chat about a new publishing scheme for this package. In #249, there was some discussion about renaming the project rescript-urql. I support this change with a few suggestions:

  • Project renamed to rescript-urql.
  • Package published as @urql/rescript — this better aligns with our publishing scopes for vue, svelte, preact, etc. even though this package still lives separately in this repo.

I'd be curious for thoughts / feelings on the above, and also advice on what our deprecation strategy for the reason-urql namespace should be.

Lastly, there was some discussion in #253 about separating out bindings for @urql/core so that ReScript users can access just those. I like the sound of this change, but I'm not fully sure about the scope of what that looks like ("Hello monorepo?"). So, some food for thought!

BREAKING CHANGE: reason-react will no longer be supported
@gaku-sei
Copy link

gaku-sei commented Mar 10, 2021

Thank you very much @parkerziegler!! I started this some days ago but you beat me here 😄

Just so you know it seems there is a dirty bug introduced in BS 9.x linked to some optimization with the code generated for pattern matching. The bug is fixed on master and version 9.0.2 is on its way, you might want to update?

I'll try to test this branch out 😄

Edit: Seems to work well on small projects 👍

@parkerziegler
Copy link
Contributor Author

Thanks for the heads up @gaku-sei! I'll wait until v9.0.2 lands. I am planning on also renaming this package to @urql/rescript for a v4.0.0 (and to bring it more consistently under the family of urql packages). So this won't be out and available for a little bit! Thanks for testing the branch early though!

package.json Outdated
"graphql": "^15.4.0",
"npm-run-all": "^4.1.5",
"react": "^16.8.0",
"reason-react": "^0.9.1",
"urql": "~1.11.2"
"urql": "~2.0.1"
Copy link

Choose a reason for hiding this comment

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

I'd love for this to switch over to a simple minor-range, since future changes shouldn't affect the bindings. I think the patch range was rather temporary, if I'm not mistaken, to prevent other changes from interferring with the bindings slowly catching up, so since we're all caught up now it should be safe to bump this to the minor range

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, you're right! I agree that a more permissive minor range sounds appropriate!

"graphql": "^0.11.0 || ^0.12.0 || ^0.13.0 || ^14.0.0 || ^15.0.0",
"react": "^16.8.0",
"reason-react": "^0.7.0",
"urql": "~1.11.0"
"urql": "~2.0.0"
},
"resolutions": {
"wonka": "5.0.0-rc.1"
Copy link

Choose a reason for hiding this comment

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

I suppose, this is still a permanent fix that all users need to add, right? Let's chat about how we can cleanly migrate all of urql to a new version, since I kind of got stuck on updating everything

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing — yeah the internals of how ReScript compiles variants really :rekt: us. I'll see if I can snag some of your time later in March (on PTO next week) to discuss what an upgrade strategy might look like.

@parkerziegler parkerziegler merged commit d441e10 into main Mar 21, 2021
@parkerziegler parkerziegler deleted the feat/support-rescript-react-urql-2 branch March 21, 2021 16:39
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.

Using @rescript/react
3 participants