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

ReasonReact (ReasonML) #196

Closed
3 of 5 tasks
jihchi opened this issue Apr 7, 2018 · 25 comments
Closed
3 of 5 tasks

ReasonReact (ReasonML) #196

jihchi opened this issue Apr 7, 2018 · 25 comments
Labels

Comments

@jihchi
Copy link

jihchi commented Apr 7, 2018

Progress

  • 🏁 Fork the starter repo & post the link in this
  • 🎨 Create logo for repo & update issue status
  • 🔨 Implement all of Conduit's functionality per the spec & API
  • 👀 Move repo to main org & Peer review final codebase by admins/community (RFC)
  • 🎉 Tag v1 release and officially list it on the README!

Current Status

Codebase (done):
https://github.com/jihchi/reason-react-realworld-example-app

@jihchi jihchi changed the title ReasonReact (Reason) ReasonReact (ReasonML) Apr 7, 2018
@Cameron-C-Chapman
Copy link
Member

Cameron-C-Chapman commented Apr 7, 2018

Excited to see this @jihchi! I've been hearing a lot about ReasonML lately.

@jihchi
Copy link
Author

jihchi commented Apr 12, 2018

New logo: logo

@jihchi
Copy link
Author

jihchi commented May 18, 2018

Hi @Cameron-C-Chapman , The codebase is done.

@anishkny
Copy link
Member

@jihchi awesome! Do you think you could fill out README sections on How it works, getting started etc? Also, would be great if there was a hosted demo we could look at?

Cheers!

@jihchi
Copy link
Author

jihchi commented May 18, 2018

@anishkny Wow, thank you for so fast response.

I'm working README, unit test and hosted demo.
I'll update here when its done.

@jihchi
Copy link
Author

jihchi commented May 18, 2018

@anishkny I've completed README, minimum unit test and hosted demo.

@anishkny
Copy link
Member

anishkny commented May 18, 2018

Great. Played around a little bit and looks good. For some reason I am seeing different fonts compared to the React/Redux Demo, but rest seems awesome.

@Cameron-C-Chapman @EricSimons PTAL!

@anishkny anishkny added rfc and removed wip labels May 18, 2018
@jihchi
Copy link
Author

jihchi commented May 19, 2018

@anishkny I forgot to delete unused CSS. Its back to normal. Thank you.

@jihchi
Copy link
Author

jihchi commented May 19, 2018

I haven't found #136 before (tried search but didn't see it).

@Cameron-C-Chapman
Copy link
Member

Cameron-C-Chapman commented May 19, 2018

Looks good to me too! Nice work @jihchi!

I didn't realize there was 2 ReasonML implementations in progress. Looking at the issues it looks like it was complete awhile ago also. 😬

@jihchi you mind taking a look and seeing if there is anything different between the 2 repos approaches? We should probably add them both to the README so it would be nice if there was some difference (version, architecture), etc that we could differentiate the 2 somehow.

It also looks like that repo is already merged into the gothinkster org, it just never got listed on the readme.

@jihchi
Copy link
Author

jihchi commented May 20, 2018

@Cameron-C-Chapman

There are some different between tech stack:

Tool gothinkster/reasonml-realworld-example-app jihchi/reason-react-realworld-example-app
Version of ReasonReact ^0.2.4 ^0.4.1
Version of BuckleScript 2.0 3.0
JS Bundler / Dev-server Custom Webpack config react-scripts@next
Client-side Routing bs-director ReasonReact built-in router
Testing bs-jest bs-jest + Enzyme
Form & Validation Custom re-formality
Storage of authorization token LocalStorage Cookie

I think gothinkster/reasonml-realworld-example-app is a little of out-dated.

@anishkny
Copy link
Member

Pinged the ReasonML Discord to get a review!

@Madsn
Copy link

Madsn commented Jul 28, 2018

I tried cloning and running the repo, exactly as described in the readme - I'm getting a lot of CRA linting warnings on the generated JS.

e.g.

<snip>
[ReasonML] >>>> Finish compiling
[CRA] Failed to compile.
[CRA]
[CRA] ./src/Profile.bs.js
[CRA] Compiling...
[CRA] Compiled with warnings.
[CRA]
[CRA] ./src/API.bs.js
[CRA] /d/repo/reason-react-realworld-example-app/src/API.bs.js
[CRA]   2:1  warning  'use strict' is unnecessary inside of modules  strict
[CRA]
[CRA] ✖ 1 problem (0 errors, 1 warning)
[CRA]   0 errors, 1 warning potentially fixable with the `--fix` option.
[CRA]
[CRA]
[CRA] ./src/App.bs.js
[CRA] /d/repo/reason-react-realworld-example-app/src/App.bs.js
<snip>

@jihchi
Copy link
Author

jihchi commented Jul 28, 2018

@Madsn

Yes, It works as expected. It's just warning, you still could start & build the project.

@Madsn
Copy link

Madsn commented Jul 28, 2018

I know it still builds, but why not prevent linting of the generated .bs.js files? Add an eslintignore perhaps?

@jihchi
Copy link
Author

jihchi commented Jul 29, 2018

@Madsn

eslintignore won't works. see here.

@Madsn
Copy link

Madsn commented Jul 29, 2018

@jihchi that just makes me sad :(. I don't get why CRA doesn't support eslint customization.

A bucklescript hook to add eslint ignore comment to the top of the file might remove the linting errors, depending on if the hook is run before or after the file is actually saved to disk and then caught by the linter.

Not ideal for sure, would be a bit of a performance hit, but might be less of a performance cost than linting the generated files.

https://bucklescript.github.io/docs/en/build-configuration.html#js-post-build

@jihchi
Copy link
Author

jihchi commented Jul 29, 2018

@Madsn Thanks! I'll take a look at it!

@jpeg729
Copy link

jpeg729 commented Oct 31, 2018

Works only partially.

I cloned, built and ran it. Ok.
I tried signing up and got the following error messages displayed.

  • Username can't be blank
  • Email can't be blank
  • Password can't be blank

They weren't blank.

@jihchi
Copy link
Author

jihchi commented Oct 31, 2018

@jpeg729 I'll take a look later. Thanks for your report.

@jihchi
Copy link
Author

jihchi commented Dec 4, 2018

@jpeg729 I've fixed and deployed. thanks for your feedback.

@forrest-akin
Copy link

forrest-akin commented Jun 27, 2019

Hi, just checking in. The project looks like it's more or less ready to merge, but there hasn't been any activity here in awhile. What are the next steps? Thanks!

@jihchi
Copy link
Author

jihchi commented Jun 28, 2019

Hi @forrest-akin ,

Currently, I'm working on upgrading re-formality to latest version.
My goal is migrating codebase to use React Hooks.
You can see it at this branch, It's still a work in progress.


2020/5/21 edit:

The codebase has been updated and it is now based on React Hooks.

It is using latest version of React and BuckleScript.

@anishkny anishkny added wip and removed rfc labels Sep 1, 2019
@dmitrii-esin
Copy link

Is it relevant?
I don't see the app in the list on the main page.

@geromegrignon
Copy link
Contributor

Hello we are currently moving all work in progress to Github Discussions.
I'll close this issue by now, you can find instruction to open a discussion there : #633

Thanks for your contribution !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants