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

Add Bucklescript bindings for Reasonml #360

Closed
MarcCoquand opened this issue May 12, 2019 · 10 comments · Fixed by #1128
Closed

Add Bucklescript bindings for Reasonml #360

MarcCoquand opened this issue May 12, 2019 · 10 comments · Fixed by #1128
Labels
🚀 Feature Request good first issue easy issue to start with - contributions welcome help wanted contributions welcome

Comments

@MarcCoquand
Copy link

fast-check is a very nice library. It would be even better if we could use it with Facebook's ReasonML instead of Typescript!

It might be possible to use https://github.com/rrdelaney/ReasonablyTyped to make the conversion easier.

@dubzzz
Copy link
Owner

dubzzz commented May 12, 2019

Hi @MarcCoquand,

I'll have a look to it. If the conversion works fine I will definitely go for it.

I know some other libs like jsverify comes with another package manually written to handle such typings bs-jsverify. But if the conversion can be done easily and automatically I can definitely add it into my build process.

@dubzzz dubzzz added feature good first issue easy issue to start with - contributions welcome help wanted contributions welcome labels May 12, 2019
dubzzz added a commit that referenced this issue May 12, 2019
Just a simple Proof-of-Concept related to the issue #360.
For the moment, it seems that the generated files are not as precise as I would like:
- numbers are translated into float (while most of the time the should be marked as int)
- reasonably-typed is failing and producing empty files in some cases
- reasonably-typed produces lots of any while typying is well-defined in ts
- reasonably-typed is not well packaged
- reasonably-typed has not been updated for a while
@TheSpyder
Copy link
Contributor

Manual bindings are a pain, but ReasonablyTyped looks abandoned. I'm hoping to switch my ReasonML project from jsverify to fast-check sometime this year, if I do I'll work on some bindings (probably base them off bs-jsverify) and make a PR. Even if it's not complete API coverage it'll still give us something to work with!

@TheSpyder
Copy link
Contributor

I've written some bindings, they seem to work well, we have migrated our bs-jsverify tests to them and it was very easy 😁

They're external bindings at the moment. I'll work on a PR to include them in fast-check directly so they are easier to keep up to date. Might take me a bit more time to do, I also need to make a decision about the map and filter functions (those function names are very common in reasonml code so having them in scope from my bindings can cause conflicts).

I'm currently thinking I'll put them in a dedicated module where they will conflict less. I've tried to structure the bindings fairly close to the documentation so everything is easy to find, but maybe functions from the advanced arbitraries documentation can go in a separate Derive module 🤔

@stale
Copy link

stale bot commented Aug 28, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 28, 2020
@TheSpyder
Copy link
Contributor

Yeah, I'm not surprised. Sorry I haven't had time to look at it since January, this year has been crazy. I can confirm however that my bindings are working very well. I still consider them temporary and would prefer to contribute them eventually.

@stale stale bot removed the stale label Aug 28, 2020
@stale
Copy link

stale bot commented Oct 27, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 27, 2020
@TheSpyder
Copy link
Contributor

@dubzzz I've had a poke around how to include my bindings into fast-check directly and it looks like BuckleScript (now ReScript) bindings are a bit of a challenge to co-locate in the project they're binding to.

There are a few problems, but the real blocker is that ReScript doesn't have a way to use separate require paths for CJS and ES6 compile modes. It can compile in either style but the require path is fixed in the source file. This isn't a problem for external references, require('fast-check') and import * from 'fast-check' work equally well, but for internal references it would need to change from ../lib/fast-check.js to ../lib/esm/fast-check.js.

For the moment I'm happy to maintain my separate project and start publishing it to NPM. I also plan to discuss ways to make this easier with the ReScript team. If it later becomes possible to integrate the bindings I can lodge a PR then, if you'd like to close this issue for now.

@dubzzz
Copy link
Owner

dubzzz commented Oct 28, 2020

If we leave the project outside of fast-check (which seems to be a good option for the moment), you might be interested into adding a small note about it in fast-check's documentation (maybe in the Tips section or next to the compatibility table).

Please feel free to add such information.

@stale stale bot removed the stale label Oct 28, 2020
@TheSpyder
Copy link
Contributor

Thanks! Once I’ve started publishing to NPM I’ll definitely do that.

@TheSpyder
Copy link
Contributor

I'll make a PR soon, but for now the package has been published. I decided to put a bit of effort into documenting how to use it 🙂
https://www.npmjs.com/rescript-fast-check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 Feature Request good first issue easy issue to start with - contributions welcome help wanted contributions welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants