Skip to content

Commit 20bff84

Browse files
committed
Address Rspack v2 review follow-ups
1 parent 74f4ed4 commit 20bff84

4 files changed

Lines changed: 114 additions & 8 deletions

File tree

lib/shakapacker/doctor.rb

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -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

@@ -1036,11 +1040,18 @@ def package_json_dependency_version(name)
10361040
end
10371041

10381042
def package_version_below?(package_name, minimum_version)
1043+
package_version_status(package_name, minimum_version).values.any?
1044+
end
1045+
1046+
def package_version_status(package_name, minimum_version)
10391047
minimum = Gem::Version.new(minimum_version)
10401048
declared = package_version_from_specifier(package_json_dependency_version(package_name))
10411049
installed = installed_package_version(package_name)
10421050

1043-
[declared, installed].compact.any? { |version| version < minimum }
1051+
{
1052+
declared_below: declared && declared < minimum,
1053+
installed_below: installed && installed < minimum
1054+
}
10441055
end
10451056

10461057
def package_version_from_specifier(version)
@@ -1132,9 +1143,7 @@ def check_optional_dependency(package_name, warnings_array, description)
11321143
def package_installed?(package_name)
11331144
return false unless package_json_exists?
11341145

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

11401149
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: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ def capture_stdout
591591

592592
it "adds an unsupported manifest plugin version issue" do
593593
doctor.send(:check_peer_dependencies)
594-
expect(doctor.issues).to include(match(/rspack-manifest-plugin \^5\.2\.2/))
594+
expect(doctor.issues).to include(match(/Declared rspack-manifest-plugin range.*\^5\.2\.2/))
595595
end
596596
end
597597

@@ -619,7 +619,8 @@ def capture_stdout
619619
it "reports the stale declared ranges" do
620620
doctor.send(:check_peer_dependencies)
621621
expect(doctor.issues).to include(match(/Shakapacker supports Rspack v2 only/))
622-
expect(doctor.issues).to include(match(/rspack-manifest-plugin \^5\.2\.2/))
622+
expect(doctor.issues).to include(match(/Declared rspack-manifest-plugin range.*\^5\.2\.2/))
623+
expect(doctor.issues).not_to include(match(/Unsupported rspack-manifest-plugin version/))
623624
end
624625
end
625626
end
@@ -2264,6 +2265,36 @@ def expect_paths_to_agree
22642265
end
22652266
end
22662267

2268+
context "with package in peerDependencies" do
2269+
before do
2270+
package_json = {
2271+
"peerDependencies" => {
2272+
"webpack" => "^5.0.0"
2273+
}
2274+
}
2275+
File.write(package_json_path, JSON.generate(package_json))
2276+
end
2277+
2278+
it "returns true" do
2279+
expect(doctor.send(:package_installed?, "webpack")).to be true
2280+
end
2281+
end
2282+
2283+
context "with package in optionalDependencies" do
2284+
before do
2285+
package_json = {
2286+
"optionalDependencies" => {
2287+
"webpack" => "^5.0.0"
2288+
}
2289+
}
2290+
File.write(package_json_path, JSON.generate(package_json))
2291+
end
2292+
2293+
it "returns true" do
2294+
expect(doctor.send(:package_installed?, "webpack")).to be true
2295+
end
2296+
end
2297+
22672298
context "with package not installed" do
22682299
before do
22692300
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)