Plots2: Remove React/Webpacker commands from install steps? (all effects already included in git/yarn)

[jywarren]

@icarito and I closely reviewed the webpacker and react install steps that were recently added to different parts of our system here, in the following PRs/issues and maybe more:

• https://github.com/publiclab/plots2/issues/9392
• https://github.com/publiclab/plots2/pull/9176
• https://github.com/publiclab/plots2/issues/9382
• https://github.com/publiclab/plots2/blob/999264ebe70d750f1836a4b4dcbdb7118595639b/.github/workflows/tests.yml#L49

We believe they add a few static files which are now part of our repo in /app/assets/... and some modifications to our package.json and yarn.lock files which are managed by those systems, and so we think there may be no further need for these install commands -- we're removing them from production deployment in https://github.com/publiclab/plots2/pull/9501 for example.

We're wondering if we should remove this from the README as well as other places around the system since it seems like they may be redundant now that all the changes are managed in either Git or Yarn.

Thoughts on this?

An in-depth discussion happened in https://github.com/publiclab/plots2/issues/9382 so I'm wondering how we follow up on that.

Is it possible that the fix for that, which re-ordered steps in https://github.com/publiclab/plots2/pull/9395, was not right? And that instead we should have removed the step entirely?

I know @noi5e is a bit offline right now and there's no big rush to this. Just making sure we don't forget to think this through and make adjustments if needed. cc @RuthNjeri @cesswairimu @waridrox and others who've been involved in the conversation! Thanks!

[RuthNjeri]

Thanks, Jeff! As For the README steps in running the Web packer commands, I noticed that each time I run the command, it updates the Package.json file and Yarn, which might be confusing when pushing code that does not touch on the React Files...It's like if running bundle install would update the gem lock file, this would cause a situation where gems are updated due to the local system of a user and not the application...

[jywarren]

Yes, i think that also aligns with our understanding that all of the effects of running those commands is reflected in the package.json and yarn.lock files, so actually running them again and again is not necessary, but ALSO that now that those changes have been git tracked, even new users of the code need not run them as long as they run yarn install as they would already normally. Does that seem right to you?

[waridrox]

Agreed with @RuthNjeri 🙌 , running the cmds does actually update the package.json and yarn.lock files. But if yarn install can already take care about those dependencies then I believe the natural intuition would be to remove the cmds...

[jywarren]

OK, thanks both of you. It changes package.json and yarn.lock, but
presumably it doesn't keep adding new things we need, right? Those were
already added in the original PR by @noi5e, i believe?

In any case I'll check back in with @icarito on this and perhaps we'll move
ahead with removing them. Thanks!!

On Fri, Apr 16, 2021 at 4:40 PM Mohammad Warid @.*>
wrote:

Agreed with @RuthNjeri https://github.com/RuthNjeri 🙌 , running the
cmds does actually update the package.json and yarn.lock files. But if yarn
install can already take care about those dependencies then I believe the
natural intuition would be to remove the cmds...

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/9502#issuecomment-821551300,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAF6J32YEVAGC5G2FXETK3TJCOCZANCNFSM4233WFSA
.

[RuthNjeri]

I was testing the yarn install command locally to see if the application will run without having to run the webpacker command...after running yarn install I needed to run bundle install to update the gems and the bin folder of the application was updated .... not sure if this is an isolated case for me...

Also, when I ran rails s, I received some webpacker errors

I had to run rails webpacker:install:reactfor the application to load...

Interestingly, the only file changes that happened was when I run bundle install. The yarn command and webpacker command did not create any file updates.

[jywarren]

Thanks @RuthNjeri -- but isn't yarn install already part of our install process?

What was it about yarn install that caused you to need to update gems?

I'm looking especially at rails g webpacker:install:react to double-check what it does:

https://github.com/reactjs/react-rails#rails-5x

https://github.com/reactjs/react-rails/blob/f8148d57a6d96b27bddfaef9f9bf26e233b27bd6/Rakefile#L26-L29

this runs yarn upgrade in the react-builds directory. @RuthNjeri is it possible that rails g webpacker:install:react simply runs yarn upgrade? or runs it in some react directory we have?

Second, i do notice a slight mismatch between what we have, rails g webpacker:install:react and the docs in react-rails, which has no g -- rails webpacker:install:react. But not sure that's a problem.

[jywarren]

Also, in general it seems like there is evolving guidance on how to completely switch from Rails Asset Pipeline to Webpacker using this gem: https://github.com/reactjs/react-rails/issues/1052 which could be good to look at.

[RuthNjeri]

Hey Jeff, addressing some of the questions now...

Thanks @RuthNjeri -- but isn't yarn install already part of our install process?
What was it about yarn install that caused you to need to update gems?
Yes, yarn install is already part of our installation process. I did try rerunning yarn install again just now and the gems were not affected, so it does seem like an isolated case or a one time thing that happens.

this runs yarn upgrade in the react-builds directory. @RuthNjeri is it possible that simply runs yarn upgrade? or runs it in some react directory we have?

Yes, I think it does run a yarn upgrade, according to this tutorial, it also makes configuration changes and creates a React component file/folder.

Second, i do notice a slight mismatch between what we have, rails g webpacker:install:react and the docs in react-rails, which has no g -- rails webpacker:install:react. But not sure that's a problem.

Therails g webpacker:install:react command seems to introduce new files and folders, like it is creating something from scratch


Come to think of it, these files and folders created are similar to the once I encountered here https://github.com/publiclab/plots2/issues/9502#issuecomment-822036027

I also ran the rails webpacker:install:react without a --g, it just seems to do some update of some sort without adding any files and folders

I suspect that rails g webpacker:install:react should only be ran once when you are initially building the app and then from then on, run it without the g command ...it could also be that the g command is not needed at all...

[RuthNjeri]

Also, in general it seems like there is evolving guidance on how to completely switch from Rails Asset Pipeline to Webpacker using this gem: reactjs/react-rails#1052 which could be good to look at.

Should we look into updating our steps to reflect this? Or should we think about a total switch to the gem?

[jywarren]

"g" may refer to "generate" if it's mirroring rake g in Rails style. So that matches your observation!

As to the asset pipeline stuff, i wonder if it's best we make the minimal intervention now, as i think Rails 6 may have a much more fully integrated solution and is coming up in the next year anyways.

So what should be our next step here? Perhaps, given that:

I suspect that rails g webpacker:install:react should only be ran once

and that after then, we get the equivalent already using yarn upgrade which is already part of our build process, maybe we just remove these lines from our steps? Because the files created by rails g webpacker:install:react are already in our git tree.

How does this sound, @noi5e @RuthNjeri?

[RuthNjeri]

Hi @jywarren

The next steps make sense, we could start out removing it from our steps, if that doesn't work, we can have the command without the g option... rails webpacker:install:react

[jywarren]

Great, thanks. @noi5e i know you have a little bandwith on this now, did you have a take on this, does it sound OK?