search

ml-in-barcelona / jsoo-react

js_of_ocaml bindings for ReactJS. Based on ReasonReact.
https://ml-in-barcelona.github.io/jsoo-react
MIT License
138 stars 19 forks source link

Add bindings to StrictMode #82

Closed jchavarri closed 1 year ago

jchavarri commented 2 years ago

https://reactjs.org/docs/strict-mode.html

schinns commented 2 years ago

Hey! This looks like a great one to start with. I have some questions before opening a PR:

Should there be a test for StrictMode? As StrictMode is a development feature, I am not sure how we could test for this. I also found it difficult to verify in the javascript output in _build. Finally, when I run make format-check, I get suggestions to add empty lines in files I did not modify. Is this expected behavior with format?

davesnx commented 2 years ago

Hello @benschinn

Thanks for jumping in! StrictMode has a bunch of "features" that might be hard to unit test. I would try to pick one that is relevant to ensure that strict mode is enabled. Since we run a browser-like environment, we might be able to detect the double rendering when effects happen? If not, I wouldn't worry too much about it.

Regarding the format-check, we run format-check on CI (https://github.com/ml-in-barcelona/jsoo-react/runs/6151024960?check_suite_focus=true#step:7:1). Maybe there's a miss-match on ocamlformat? Try running opam update or opam install . --deps-only --with-test