Skip to content

Commit

Permalink
fix(publish): handle diagnostic outside graph (#22310)
Browse files Browse the repository at this point in the history
Hacky quick fix. The real fix is a lot more work to do (move the
`SourceTextInfo` into all the diagnostics in order to make this less
error pone). I've already started on it, but it will require a lot of
downstream create changes.

Closes #22288
  • Loading branch information
dsherret committed Feb 7, 2024
1 parent 1007358 commit e546848
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 2 deletions.
27 changes: 25 additions & 2 deletions cli/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,36 @@ pub trait SourceTextStore {

pub struct SourceTextParsedSourceStore<'a>(pub LazyGraphSourceParser<'a>);

impl<'a> SourceTextParsedSourceStore<'a> {
pub fn get_source_text_from_store(
&self,
specifier: &ModuleSpecifier,
) -> Option<Cow<'_, SourceTextInfo>> {
let parsed_source = self.0.get_or_parse_source(specifier).ok()??;
Some(Cow::Owned(parsed_source.text_info().clone()))
}
}

impl SourceTextStore for SourceTextParsedSourceStore<'_> {
fn get_source_text<'a>(
&'a self,
specifier: &ModuleSpecifier,
) -> Option<Cow<'a, SourceTextInfo>> {
let parsed_source = self.0.get_or_parse_source(specifier).ok()??;
Some(Cow::Owned(parsed_source.text_info().clone()))
match self.get_source_text_from_store(specifier) {
Some(text_info) => Some(text_info),
None => {
// todo(#22117): this is extremely hacky and bad because the file
// may have changed by the time we get here. Instead of doing this,
// we should store the text info in the diagnostics
if specifier.scheme() == "file" {
let path = specifier.to_file_path().ok()?;
let text = std::fs::read_to_string(path).ok()?;
Some(Cow::Owned(SourceTextInfo::new(text.into())))
} else {
None
}
}
}
}
}

Expand Down
26 changes: 26 additions & 0 deletions cli/tests/integration/publish_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,32 @@ fn publish_non_exported_files_using_import_map() {
.any(|l| l.contains("Unfurling") && l.ends_with("other.ts")));
}

#[test]
fn publish_warning_not_in_graph() {
let context = publish_context_builder().build();
let temp_dir = context.temp_dir().path();
temp_dir.join("deno.json").write_json(&json!({
"name": "@foo/bar",
"version": "1.0.0",
"exports": "./mod.ts",
}));
// file not in the graph that uses a non-analyzable dynamic import (cause a diagnostic)
let other_ts = temp_dir.join("_other.ts");
other_ts
.write("const nonAnalyzable = './_other.ts'; await import(nonAnalyzable);");
let mod_ts = temp_dir.join("mod.ts");
mod_ts.write(
"export function test(a: number, b: number): number { return a + b; }",
);
context
.new_command()
.args("publish --token 'sadfasdf'")
.run()
.assert_matches_text(
"[WILDCARD]unable to analyze dynamic import[WILDCARD]",
);
}

itest!(javascript_missing_decl_file {
args: "publish --token 'sadfasdf'",
output: "publish/javascript_missing_decl_file.out",
Expand Down

0 comments on commit e546848

Please sign in to comment.