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

[Proposal] Give the Reagent examples some TLC #551

Open
mchughs opened this issue Nov 11, 2021 · 1 comment
Open

[Proposal] Give the Reagent examples some TLC #551

mchughs opened this issue Nov 11, 2021 · 1 comment

Comments

@mchughs
Copy link

mchughs commented Nov 11, 2021

I was going through some of the examples, in particular the react-context and the functional-components-and-hooks examples.

I noticed a lot of little issues.

Problems with both:

  • Builds are based on lein-figwheel, a deprecated build tool. With examples you want to just start them up as quickly as possible to play around with the underlying example. You don't want to fiddle with a build. I'd propose using deps.edn and shadow-cljs as I believe they are more standard nowadays, but if there is a particular affinity for lein and figwheel we could atleast move it to figwheel-main.
  • The versions for some of the dependencies are out-of-date which actually causes problems if you try to play around with new features in the example. I can bring everything up to date.
  • Should explicitly pull in react and react-dom latest versions.

Just in react-context:

  • No README.md present despite the index.html indicating there should be.
  • It's pulling in [cljsjs/react-sortable-hoc "1.11.0-0"] as a depency ostensibly to just pull in react and react-dom?
  • I think it makes sense to add a bit about react/useContext. This would require using the :>f tag too.

Just in functional-components-and-hooks:

  • The build process is more complicated than it needs to be. Am I going to be going to prod with example code?
  • The build compains it can't find react-dom if you don't include it as a dependency. (I don't see where it's being used though so it's kinda of weird. Any ideas?)

I'm happy to make an MR for these two and I can also take a look at the other ones as well. Is there anything I proposed which you disagree with?

@mchughs mchughs changed the title Give the Reagent example some TLC [Proposal] Give the Reagent example some TLC Nov 11, 2021
@mchughs mchughs changed the title [Proposal] Give the Reagent example some TLC [Proposal] Give the Reagent examples some TLC Nov 11, 2021
@Deraen
Copy link
Member

Deraen commented Dec 19, 2021

Sounds good.

Some of the examples are missing cljsjs react and react-dom dependencies as those were included in Reagent before 1.1.0.

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

No branches or pull requests

2 participants