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.

Sign up for GitHub

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

Jump to bottom

Fix install commands for different yarn versions #525

Merged
merged 1 commit into from
Nov 25, 2024
Merged

Fix install commands for different yarn versions #525

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions docs/v8_upgrade.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ If your host might differ, between various environments for example, you will ei
- Ensure the assets are specifically rebuilt for each environment (Heroku pipeline promote feature for example does not do that by default).
- Make sure the assets are compiled with `SHAKAPACKER_ASSET_HOST=''` ENV variable to avoid hardcording URLs in packs output.

Second option has got a certain gotcha - dynamic imports and static asset references (like image paths in CSS) will end up without host reference and the app will try and fetch them from your app host rather than defined `config.asset_host`.

Make sure the assets are compiled with `SHAKAPACKER_ASSET_HOST=''` ENV variable to avoid hardcoding URLs in packs output.
The second option has got a certain gotcha - dynamic imports and static asset references (like image paths in CSS) will end up without a host reference and the app will try and fetch them from your app host rather than defined `config.asset_host`.

To get around that, you can use dynamic override as outlined by [Webpack documentation](https://webpack.js.org/guides/asset-modules/#on-the-fly-override).

Expand Down Expand Up @@ -102,10 +100,10 @@ namespace :assets do
raise if File.exist?("package.json") && !(system "npm ci")

# yarn v1.x (classic)
raise if File.exist?("package.json") && !(system "yarn install --immutable")
raise if File.exist?("package.json") && !(system "yarn install --frozen-lockfile")

# yarn v2+ (berry)
raise if File.exist?("package.json") && !(system "yarn install --frozen-lockfile")
raise if File.exist?("package.json") && !(system "yarn install --immutable")
Comment on lines +103 to +106
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix Yarn install commands documentation

The current documentation might be confusing as it suggests different flags for different Yarn versions. According to Yarn's documentation:

  • For Yarn Classic (1.x): --frozen-lockfile is the correct flag
  • For Yarn Berry (2+): Both --immutable and --frozen-lockfile work (the latter for backward compatibility)

To make this clearer, consider this revision:

    # yarn v1.x (classic)
-    raise if File.exist?("package.json") && !(system "yarn install --frozen-lockfile")
+    # Use --frozen-lockfile for Yarn Classic
+    raise if File.exist?("package.json") && !(system "yarn install --frozen-lockfile")

    # yarn v2+ (berry)
-    raise if File.exist?("package.json") && !(system "yarn install --immutable")
+    # Both --immutable and --frozen-lockfile (for backward compatibility) work
+    raise if File.exist?("package.json") && !(system "yarn install --immutable")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
raise if File.exist?("package.json") && !(system "yarn install --frozen-lockfile")
# yarn v2+ (berry)
raise if File.exist?("package.json") && !(system "yarn install --frozen-lockfile")
raise if File.exist?("package.json") && !(system "yarn install --immutable")
# yarn v1.x (classic)
# Use --frozen-lockfile for Yarn Classic
raise if File.exist?("package.json") && !(system "yarn install --frozen-lockfile")
# yarn v2+ (berry)
# Both --immutable and --frozen-lockfile (for backward compatibility) work
raise if File.exist?("package.json") && !(system "yarn install --immutable")


# bun v1+
raise if File.exist?("package.json") && !(system "bun install --frozen-lockfile")
Expand Down
Loading