Minor fixes for migrating webpack to esbuild during a v2 upgrade

[ayushn21]

This is a 🐛 bug fix.

Summary

I ran into a few issues migrating from webpack to esbuild while upgrading to BT v2 on a relatively old project.

1. An error was thrown when running the migration command:

  Exception raised: NoMethodError
undefined method `add_npm_package' for an instance of Bridgetown::Commands::Esbuild
                 1: /Users/ayush/.rbenv/versions/3.3.5/lib/ruby/gems/3.3.0/gems/bridgetown-core-2.0.0.beta2/lib/bridgetown-core/commands/esbuild/migrate-from-webpack.rb:21:in `apply'
                 2: /Users/ayush/.rbenv/versions/3.3.5/lib/ruby/gems/3.3.0/gems/thor-1.3.2/lib/thor/actions.rb:231:in `instance_eval'
                 3: /Users/ayush/.rbenv/versions/3.3.5/lib/ruby/gems/3.3.0/gems/thor-1.3.2/lib/thor/actions.rb:231:in `apply'
                 4: /Users/ayush/.rbenv/versions/3.3.5/lib/ruby/gems/3.3.0/gems/bridgetown-core-2.0.0.beta2/lib/bridgetown-core/commands/esbuild.rb:60:in `block in perform'
                 5: /Users/ayush/.rbenv/versions/3.3.5/lib/ruby/gems/3.3.0/gems/thor-1.3.2/lib/thor/actions.rb:190:in `block in inside'

Fixed by 7200d4c

2. yarn remove was hardcoded in the migration automation. This failed as I was no longer using Yarn. Fixed by adding an uninstall command: 09d2be7.

3. Since I had a few spinning plates in this old project while upgrading, I had deleted yarn.lock but not yet run npm install meaning there was no lockfile. Running the migration in this state just logs out some info as shown below and completes without showing an error or anything:

Installing required packages
        gsub  package.json
         run   add -D esbuild glob postcss postcss-flexbugs-fixes postcss-preset-env postcss-import [email protected] from "."

This is because the package_manager returned is "" since there's no lockfile. I've amended the logic to return npm as the default as long as package.json exists: 5cf2fd3.

[ayushn21]

PS @jaredcwhite I think this PR is a great case against "Squash and merge" as the commits are atomic, descriptive, and isolated :D!

This is an official petition to use just merge instead of squashing as well when we can: https://binarysolo.blog/how-i-manage-my-git-history/!

[ayushn21]

I've also disabled the Rubocop rule preventing multiple includes on the same line:

include Thor::Actions, Bridgetown::Commands::Actions

I think this is a useful way to write includes in some circumstances and disallowing them across the board seems a bit heavy handed to me.

[jaredcwhite]

Looks good @ayushn21!

I've enabled merge commits for the repo so it'll work for this PR now, but the reason I generally prefer squash commits is it makes it easy for me to review the release when I go to work on the changelog and last-minute documentation, as I can breeze through the git graph and click on PR commits to see everything that went in at a glance. I think that'd be harder with just merges.

[ayushn21]

I generally prefer squash commits is it makes it easy for me to review the release when I go to work on the changelog and last-minute documentation

Ah yeah fair enough .... Wonder if there's another way to do that ... but in PRs like this I reckon it'd help to have the individual commits when building the changelog. I'm not calling for a blanket change here, just that we use both on a case-by-case basis :).

[jaredcwhite]

@ayushn21 actually, scratch that…I can still see all the changed files when looking at the standard merge commit, so actually there's no real benefit to the squash from that standpoint. So I'm fine either way. ☺️

[ayushn21]

Ah awesome 😄

On October 17, 2024, GitHub Notification ***@***.***> wrote: @ayushn21 <https://github.com/ayushn21> actually, scratch that…I can still see all the change files when looking at the standard merge commit, so actually there's no real benefit to the squash from that standpoint. So I'm fine either way. ☺️

— Reply to this email directly, view it on GitHub <#926 (comment)- 2420027658>, or unsubscribe <https://github.com/notifications/unsubscribe- auth/ABFSFB6YQFJ5V4G2MV26LUTZ37SS5AVCNFSM6AAAAABQDTOCLOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRQGAZDONRVHA>. You are receiving this because you were mentioned.Message ID: ***@***.***>