fix(lockfile): only consider package.json beside lockfile in workspace property (#22179)

Closes https://github.com/denoland/deno/issues/22176 (see detail there)
This commit is contained in:
David Sherret 2024-01-30 09:26:30 -05:00 committed by GitHub
parent b0bd4f379c
commit 0e1cae32b3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 167 additions and 20 deletions

View file

@ -293,15 +293,38 @@ impl CliFactory {
}
pub fn maybe_lockfile(&self) -> &Option<Arc<Mutex<Lockfile>>> {
fn check_no_npm(lockfile: &Mutex<Lockfile>, options: &CliOptions) -> bool {
if options.no_npm() {
return true;
}
// Deno doesn't yet understand npm workspaces and the package.json resolution
// may be in a different folder than the deno.json/lockfile. So for now, ignore
// any package.jsons that are in different folders
options
.maybe_package_json()
.as_ref()
.map(|package_json| {
package_json.path.parent() != lockfile.lock().filename.parent()
})
.unwrap_or(false)
}
self.services.lockfile.get_or_init(|| {
let maybe_lockfile = self.options.maybe_lockfile();
// initialize the lockfile with the workspace's configuration
if let Some(lockfile) = &maybe_lockfile {
let package_json_deps = self
.package_json_deps_provider()
.reqs()
.map(|reqs| reqs.into_iter().map(|s| format!("npm:{}", s)).collect())
let no_npm = check_no_npm(lockfile, &self.options);
let package_json_deps = (!no_npm)
.then(|| {
self
.package_json_deps_provider()
.reqs()
.map(|reqs| {
reqs.into_iter().map(|s| format!("npm:{}", s)).collect()
})
.unwrap_or_default()
})
.unwrap_or_default();
let mut lockfile = lockfile.lock();
let config = match self.options.maybe_workspace_config() {
@ -352,7 +375,7 @@ impl CliFactory {
};
lockfile.set_workspace_config(
deno_lockfile::SetWorkspaceConfigOptions {
no_npm: self.options.no_npm(),
no_npm,
no_config: self.options.no_config(),
config,
nv_to_jsr_url: |nv| {

View file

@ -1042,21 +1042,8 @@ fn lock_deno_json_package_json_deps() {
.run()
.skip_output_check();
let lockfile = temp_dir.join("deno.lock");
// todo(dsherret): it would be nice if the test server didn't produce
// different hashes depending on what operating system it's running on
let esm_basic_integrity = lockfile
.read_json_value()
.get("packages")
.unwrap()
.get("npm")
.unwrap()
.get("@denotest/esm-basic@1.0.0")
.unwrap()
.get("integrity")
.unwrap()
.as_str()
.unwrap()
.to_string();
let esm_basic_integrity =
get_lockfile_npm_package_integrity(&lockfile, "@denotest/esm-basic@1.0.0");
lockfile.assert_matches_json(json!({
"version": "3",
"packages": {
@ -1179,6 +1166,143 @@ fn lock_deno_json_package_json_deps() {
}));
}
#[test]
fn lock_deno_json_package_json_deps_workspace() {
let context = TestContextBuilder::new()
.use_temp_cwd()
.use_http_server()
.add_npm_env_vars()
.add_jsr_env_vars()
.build();
let temp_dir = context.temp_dir().path();
// deno.json
let deno_json = temp_dir.join("deno.json");
deno_json.write_json(&json!({}));
// package.json
let package_json = temp_dir.join("package.json");
package_json.write_json(&json!({
"workspaces": ["package-a"],
"dependencies": {
"@denotest/cjs-default-export": "1"
}
}));
// main.ts
let main_ts = temp_dir.join("main.ts");
main_ts.write("import '@denotest/cjs-default-export';");
// package-a/package.json
let a_package = temp_dir.join("package-a");
a_package.create_dir_all();
let a_package_json = a_package.join("package.json");
a_package_json.write_json(&json!({
"dependencies": {
"@denotest/esm-basic": "1"
}
}));
// package-a/main.ts
let main_ts = a_package.join("main.ts");
main_ts.write("import '@denotest/esm-basic';");
context
.new_command()
.args("run package-a/main.ts")
.run()
.skip_output_check();
let lockfile = temp_dir.join("deno.lock");
let esm_basic_integrity =
get_lockfile_npm_package_integrity(&lockfile, "@denotest/esm-basic@1.0.0");
// no "workspace" because deno isn't smart enough to figure this out yet
// since it discovered the package.json in a folder different from the lockfile
lockfile.assert_matches_json(json!({
"version": "3",
"packages": {
"specifiers": {
"npm:@denotest/esm-basic@1": "npm:@denotest/esm-basic@1.0.0"
},
"npm": {
"@denotest/esm-basic@1.0.0": {
"integrity": esm_basic_integrity,
"dependencies": {}
}
}
},
"remote": {},
}));
// run a command that causes discovery of the root package.json beside the lockfile
context
.new_command()
.args("run main.ts")
.run()
.skip_output_check();
// now we should see the dependencies
let cjs_default_export_integrity = get_lockfile_npm_package_integrity(
&lockfile,
"@denotest/cjs-default-export@1.0.0",
);
let expected_lockfile = json!({
"version": "3",
"packages": {
"specifiers": {
"npm:@denotest/cjs-default-export@1": "npm:@denotest/cjs-default-export@1.0.0",
"npm:@denotest/esm-basic@1": "npm:@denotest/esm-basic@1.0.0"
},
"npm": {
"@denotest/cjs-default-export@1.0.0": {
"integrity": cjs_default_export_integrity,
"dependencies": {}
},
"@denotest/esm-basic@1.0.0": {
"integrity": esm_basic_integrity,
"dependencies": {}
}
}
},
"remote": {},
"workspace": {
"packageJson": {
"dependencies": [
"npm:@denotest/cjs-default-export@1"
]
}
}
});
lockfile.assert_matches_json(expected_lockfile.clone());
// now run the command again in the package with the nested package.json
context
.new_command()
.args("run package-a/main.ts")
.run()
.skip_output_check();
// the lockfile should stay the same as the above because the package.json
// was found in a different directory
lockfile.assert_matches_json(expected_lockfile.clone());
}
fn get_lockfile_npm_package_integrity(
lockfile: &PathRef,
package_name: &str,
) -> String {
// todo(dsherret): it would be nice if the test server didn't produce
// different hashes depending on what operating system it's running on
lockfile
.read_json_value()
.get("packages")
.unwrap()
.get("npm")
.unwrap()
.get(package_name)
.unwrap()
.get("integrity")
.unwrap()
.as_str()
.unwrap()
.to_string()
}
itest!(mts_dmts_mjs {
args: "run subdir/import.mts",
output: "run/mts_dmts_mjs.out",