Skip to content

Commit 78c16fb

Browse files
committed
Address Rspack v2 review follow-ups
1 parent 74f4ed4 commit 78c16fb

5 files changed

Lines changed: 126 additions & 11 deletions

File tree

lib/shakapacker/bundler_switcher.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ class BundlerSwitcher
1919
prod: %w[webpack-merge]
2020
}.freeze
2121

22-
# Default dependencies for each bundler (package names only, no versions)
22+
# Default dependencies for each bundler. Rspack entries include install-time
23+
# version ranges; package_names strips them before removal and display.
2324
# Note: Excludes independent/optional dependencies like @swc/core, swc-loader (user-configured
2425
# transpilers)
2526
DEFAULT_RSPACK_DEPS = {

lib/shakapacker/doctor.rb

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ def check_rspack_peer_deps(deps)
325325
}
326326

327327
essential_rspack.each do |package, version|
328-
unless deps[package]
328+
unless deps[package] || installed_package_version(package)
329329
@issues << "Missing essential rspack dependency: #{package} (#{version})"
330330
end
331331
end
@@ -347,9 +347,13 @@ def check_rspack_peer_deps(deps)
347347
"Upgrade #{unsupported_packages.join(' and ')} to ^2.0.0."
348348
end
349349

350-
if deps["rspack-manifest-plugin"] && package_version_below?("rspack-manifest-plugin", "5.2.2")
350+
manifest_status = package_version_status("rspack-manifest-plugin", "5.2.2")
351+
if deps["rspack-manifest-plugin"] && manifest_status[:installed_below]
351352
@issues << "Unsupported rspack-manifest-plugin version: Shakapacker requires rspack-manifest-plugin " \
352353
"^5.2.2 for Rspack v2."
354+
elsif deps["rspack-manifest-plugin"] && manifest_status[:declared_below]
355+
@issues << "Declared rspack-manifest-plugin range allows unsupported versions. " \
356+
"Update package.json to require rspack-manifest-plugin ^5.2.2 for Rspack v2."
353357
end
354358
end
355359

@@ -1035,12 +1039,15 @@ def package_json_dependency_version(name)
10351039
declared_package_dependencies(read_package_json)[name]
10361040
end
10371041

1038-
def package_version_below?(package_name, minimum_version)
1042+
def package_version_status(package_name, minimum_version)
10391043
minimum = Gem::Version.new(minimum_version)
10401044
declared = package_version_from_specifier(package_json_dependency_version(package_name))
10411045
installed = installed_package_version(package_name)
10421046

1043-
[declared, installed].compact.any? { |version| version < minimum }
1047+
{
1048+
declared_below: declared && declared < minimum,
1049+
installed_below: installed && installed < minimum
1050+
}
10441051
end
10451052

10461053
def package_version_from_specifier(version)
@@ -1132,9 +1139,7 @@ def check_optional_dependency(package_name, warnings_array, description)
11321139
def package_installed?(package_name)
11331140
return false unless package_json_exists?
11341141

1135-
package_json = read_package_json
1136-
dependencies = (package_json["dependencies"] || {}).merge(package_json["devDependencies"] || {})
1137-
dependencies.key?(package_name)
1142+
declared_package_dependencies(read_package_json).key?(package_name)
11381143
end
11391144

11401145
def package_json_exists?

package/environments/development.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* @module environments/development
44
*/
55

6+
import type { RuleSetUseItem } from "webpack"
67
import type {
78
WebpackConfigWithDevServer,
89
RspackConfigWithDevServer
@@ -16,6 +17,21 @@ const webpackDevServerConfig = require("../webpackDevServerConfig")
1617
const { runningWebpackDevServer } = require("../env")
1718
const { moduleExists } = require("../utils/helpers")
1819

20+
let rspackReactRefreshTransformEnabled = false
21+
22+
type SwcLoaderUse = {
23+
loader?: string
24+
options?: {
25+
jsc?: {
26+
transform?: {
27+
react?: {
28+
refresh?: boolean
29+
}
30+
}
31+
}
32+
}
33+
}
34+
1935
/**
2036
* Base development configuration shared between webpack and rspack
2137
*/
@@ -85,6 +101,7 @@ const rspackDevConfig = (): RspackConfigWithDevServer => {
85101
"[SHAKAPACKER WARNING] Could not resolve a constructor from @rspack/plugin-react-refresh; React Refresh will be skipped in development."
86102
)
87103
} else {
104+
rspackReactRefreshTransformEnabled = true
88105
rspackConfig.plugins = [
89106
...(rspackConfig.plugins || []),
90107
new ReactRefreshRspackPlugin()
@@ -95,7 +112,39 @@ const rspackDevConfig = (): RspackConfigWithDevServer => {
95112
return rspackConfig
96113
}
97114

115+
const enableRspackReactRefreshTransform = (
116+
environmentConfig: RspackConfigWithDevServer
117+
) => {
118+
const rules = environmentConfig.module?.rules
119+
if (!Array.isArray(rules)) return
120+
121+
rules.forEach((rule) => {
122+
if (!rule || typeof rule !== "object" || !("use" in rule)) return
123+
124+
const ruleWithUse = rule as { use?: RuleSetUseItem | RuleSetUseItem[] }
125+
const loaders = Array.isArray(ruleWithUse.use)
126+
? ruleWithUse.use
127+
: [ruleWithUse.use]
128+
129+
loaders.forEach((loader) => {
130+
if (!loader || typeof loader !== "object" || !("loader" in loader)) return
131+
132+
const loaderConfig = loader as SwcLoaderUse
133+
if (loaderConfig.loader === "builtin:swc-loader") {
134+
const reactTransform = loaderConfig.options?.jsc?.transform?.react
135+
if (reactTransform) reactTransform.refresh = true
136+
}
137+
})
138+
})
139+
}
140+
98141
const bundlerConfig =
99142
config.assets_bundler === "rspack" ? rspackDevConfig() : webpackDevConfig()
100143

101-
module.exports = merge(baseConfig, bundlerConfig)
144+
const environmentConfig = merge(baseConfig, bundlerConfig)
145+
146+
if (config.assets_bundler === "rspack" && rspackReactRefreshTransformEnabled) {
147+
enableRspackReactRefreshTransform(environmentConfig)
148+
}
149+
150+
module.exports = environmentConfig

spec/shakapacker/doctor_spec.rb

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,18 @@ def capture_stdout
533533
"Missing essential rspack dependency: @rspack/dev-server (^2.0.0)"
534534
)
535535
end
536+
537+
it "does not add a missing issue when dev-server is installed" do
538+
dev_server_pkg = root_path.join("node_modules/@rspack/dev-server/package.json")
539+
FileUtils.mkdir_p(dev_server_pkg.dirname)
540+
File.write(dev_server_pkg, JSON.generate({ "name" => "@rspack/dev-server", "version" => "2.0.0" }))
541+
542+
doctor.send(:check_peer_dependencies)
543+
544+
expect(doctor.issues).not_to include(
545+
"Missing essential rspack dependency: @rspack/dev-server (^2.0.0)"
546+
)
547+
end
536548
end
537549

538550
context "with essential rspack dependencies declared as peers" do
@@ -591,7 +603,7 @@ def capture_stdout
591603

592604
it "adds an unsupported manifest plugin version issue" do
593605
doctor.send(:check_peer_dependencies)
594-
expect(doctor.issues).to include(match(/rspack-manifest-plugin \^5\.2\.2/))
606+
expect(doctor.issues).to include(match(/Declared rspack-manifest-plugin range.*\^5\.2\.2/))
595607
end
596608
end
597609

@@ -619,7 +631,8 @@ def capture_stdout
619631
it "reports the stale declared ranges" do
620632
doctor.send(:check_peer_dependencies)
621633
expect(doctor.issues).to include(match(/Shakapacker supports Rspack v2 only/))
622-
expect(doctor.issues).to include(match(/rspack-manifest-plugin \^5\.2\.2/))
634+
expect(doctor.issues).to include(match(/Declared rspack-manifest-plugin range.*\^5\.2\.2/))
635+
expect(doctor.issues).not_to include(match(/Unsupported rspack-manifest-plugin version/))
623636
end
624637
end
625638
end
@@ -2264,6 +2277,36 @@ def expect_paths_to_agree
22642277
end
22652278
end
22662279

2280+
context "with package in peerDependencies" do
2281+
before do
2282+
package_json = {
2283+
"peerDependencies" => {
2284+
"webpack" => "^5.0.0"
2285+
}
2286+
}
2287+
File.write(package_json_path, JSON.generate(package_json))
2288+
end
2289+
2290+
it "returns true" do
2291+
expect(doctor.send(:package_installed?, "webpack")).to be true
2292+
end
2293+
end
2294+
2295+
context "with package in optionalDependencies" do
2296+
before do
2297+
package_json = {
2298+
"optionalDependencies" => {
2299+
"webpack" => "^5.0.0"
2300+
}
2301+
}
2302+
File.write(package_json_path, JSON.generate(package_json))
2303+
end
2304+
2305+
it "returns true" do
2306+
expect(doctor.send(:package_installed?, "webpack")).to be true
2307+
end
2308+
end
2309+
22672310
context "with package not installed" do
22682311
before do
22692312
File.write(package_json_path, JSON.generate({}))

test/package/environments/development-rspack-react-refresh.test.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,12 @@ const hasReactRefreshPluginInstance = (environmentConfig) => {
4747
)
4848
}
4949

50+
const swcReactTransforms = (environmentConfig) =>
51+
(environmentConfig.module?.rules || [])
52+
.flatMap((rule) => (Array.isArray(rule.use) ? rule.use : []))
53+
.filter((loader) => loader.loader === "builtin:swc-loader")
54+
.map((loader) => loader.options.jsc.transform.react)
55+
5056
describe("Rspack React refresh development config", () => {
5157
afterEach(() => {
5258
jest.restoreAllMocks()
@@ -72,6 +78,17 @@ describe("Rspack React refresh development config", () => {
7278
).toBe(true)
7379
})
7480

81+
test("enables the SWC React refresh transform when the plugin is loaded", () => {
82+
const environmentConfig = loadRspackDevelopmentConfig()
83+
84+
expect(swcReactTransforms(environmentConfig)).toStrictEqual(
85+
expect.arrayContaining([
86+
expect.objectContaining({ refresh: true }),
87+
expect.objectContaining({ refresh: true })
88+
])
89+
)
90+
})
91+
7592
test("skips the legacy direct CommonJS export shape", () => {
7693
const warn = jest.spyOn(console, "warn").mockImplementation(() => {})
7794
function ReactRefreshRspackPlugin() {}

0 commit comments

Comments
 (0)