some comments

Reviewed 15 of 15 files at r1, 1 of 1 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @gscarv13)

.prettierignore, line 11 at r2 (raw file):

spec/react_on_rails/dummy-for-generators/app/javascript/bundles/HelloWorld/*
bundle/
lib/generators/react_on_rails/templates/base/base/config/webpack

I don't understand this change

lib/generators/react_on_rails/base_generator.rb, line 67 at r1 (raw file):

        base_files = %w[config/webpacker.yml]
        base_files.each { |file| copy_file("#{base_path}#{file}", file) }
      end

Please push up a sample public repo with the generator run.

lib/generators/react_on_rails/base_generator.rb, line 95 at r1 (raw file):

        puts "Adding dev dependencies"
        run "yarn add -D @pmmmwh/react-refresh-webpack-plugin fork-ts-checker-webpack-plugin react-refresh"

@Judahmeek we can leave the versions unlocked until the generator fails due to a new version breaking things.

lib/generators/react_on_rails/templates/base/base/babel.config.js, line 2 at r1 (raw file):

// The source code including full typescript support is available at:
// https://github.com/shakacode/react_on_rails_tutorial_with_ssr_and_hmr_fast_refresh/blob/master/babel.config.js

Should we invite people to join the slack to discuss installation/upgrade issues?

lib/generators/react_on_rails/templates/base/base/babel.config.js, line 5 at r1 (raw file):

module.exports = function (api) {
  const validEnv = ['development', 'test', 'production'];

we could customize the version here: https://github.com/rails/webpacker/blob/master/package/babel/preset.js

we could invoke the function and modify the returned object

lib/generators/react_on_rails/templates/base/base/app/javascript/bundles/HelloWorld/components/HelloWorld.module.css, line 2 at r1 (raw file):

.bold {
    color: green;

name the style

bright

lib/generators/react_on_rails/templates/base/base/config/webpacker.yml, line 62 at r1 (raw file):

  # Cache manifest.json for performance
  cache_manifest: true

this file is installed by rails/webpacker. Not sure we should be overwriting it.

Reviewable status: 14 of 16 files reviewed, 7 unresolved discussions (waiting on @gscarv13 and @justin808)

.prettierignore, line 11 at r2 (raw file):

Prettier and ESLint rules are conflicting and breaking the CI, I thought it was better to ignore prettier in this case.

lib/generators/react_on_rails/base_generator.rb, line 67 at r1 (raw file):

https://github.com/gscarv13/demo-update-webpacker-config

lib/generators/react_on_rails/templates/base/base/config/webpacker.yml, line 62 at r1 (raw file):

I added this initially because there are some small changes compared to the one that is installed by default, you can see the diff here https://www.diffchecker.com/ebXd6vc2

Maybe keep the original and mention the changes necessary to make it work on the tutorial?

Reviewed 2 of 2 files at r4, 11 of 11 files at r5, all commit messages.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @gscarv13 and @justin808)

lib/generators/react_on_rails/base_generator.rb, line 98 at r5 (raw file):

        puts "Adding default javascript Rails dependencies"
        run " yarn add @rails/ujs turbolinks @rails/activestorage"

This makes no sense at all.

Seems totally unnecessary. Did you see https://github.com/shakacode/react_on_rails/blob/master/docs/guides/tutorial.md ?

Why are you adding these?

lib/generators/react_on_rails/base_generator.rb, line 101 at r5 (raw file):

        puts "Adding dev dependencies"
        run "yarn add -D @pmmmwh/react-refresh-webpack-plugin fork-ts-checker-webpack-plugin react-refresh"

base generator should not have TS

it's the basics

lib/generators/react_on_rails/templates/base/base/babel.config.js, line 6 at r5 (raw file):

module.exports = function (api) {
  // eslint-disable-next-line global-require
  const defaultConfigFunc = require('@rails/webpacker/package/babel/preset.js');

why is the require inside of the function?

I think there is no reason it can't be outside.

lib/generators/react_on_rails/templates/base/base/Procfile.dev, line 5 at r5 (raw file):

rails: bundle exec rails s -p 3000
wp-client: bin/webpack-dev-server
wp-server: SERVER_BUNDLE_ONLY=yes bin/webpack --watch

check the red chars at the line ending

please configure editor correctly

Reviewed 6 of 6 files at r6, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @gscarv13)

lib/generators/react_on_rails/base_generator.rb, line 67 at r1 (raw file):

Please add this to the main PR comment.

Can you create a loom video walkthrough of the tutorial.md steps?

lib/generators/react_on_rails/base_generator.rb, line 98 at r5 (raw file):

OK

@gscarv13 please mark comments as resolved for me next time.

lib/generators/react_on_rails/templates/base/base/babel.config.js, line 2 at r1 (raw file):

@gscarv13 can we have the install generator insert a template here?

That way we have ONE copy of the header.

https://dev.to/vinistock/understanding-and-writing-rails-generators-10h1
see the .tt files
https://arsfutura.com/magazine/diy-create-your-own-rails-generator/

template "service.erb", generator_path
that will copy the file and expand templates (erb)

lib/generators/react_on_rails/templates/base/base/config/webpacker.yml, line 62 at r1 (raw file):

You're 100% right. We should overwrite it if one is going to use the generator.

lib/generators/react_on_rails/templates/base/base/Procfile.dev-hmr, line 1 at r6 (raw file):

# Procfile for development using HMR

this is not right

you have this reversed and we should update the docs

procfile.dev ==> HMR ==> default
procfile.dev-static ==> static files, not default

Reviewable status: 9 of 24 files reviewed, 6 unresolved discussions (waiting on @gscarv13 and @justin808)

lib/generators/react_on_rails/templates/base/base/Procfile.dev, line 5 at r5 (raw file):

Done.

lib/generators/react_on_rails/templates/base/base/Procfile.dev-hmr, line 1 at r6 (raw file):

Done.

lib/generators/react_on_rails/templates/base/base/app/javascript/bundles/HelloWorld/components/HelloWorld.module.css, line 2 at r1 (raw file):

Done.

lib/generators/react_on_rails/templates/base/base/config/webpacker.yml, line 62 at r1 (raw file):

Done.

lib/generators/react_on_rails/templates/base/base/babel.config.js, line 2 at r1 (raw file):

Done.

Reviewable status: 9 of 24 files reviewed, 6 unresolved discussions (waiting on @justin808)

lib/generators/react_on_rails/base_generator.rb, line 67 at r1 (raw file):

Done.

Reviewed 15 of 15 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @gscarv13)

Reviewed 4 of 6 files at r16, 2 of 2 files at r17, 4 of 5 files at r18, 1 of 1 files at r19, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gscarv13)

I'll merge and release soon.

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gscarv13)