Skip to content

Commit

Permalink
fix: support browser filed false value (#395)
Browse files Browse the repository at this point in the history
* fix: support browser filed false value

* chore: add ResolvedPath

* Fix snapshot

* Fix clippy

---------

Co-authored-by: Yunfei <[email protected]>
  • Loading branch information
underfin and hyf0 authored Jan 24, 2024
1 parent 683416a commit c3a82a4
Show file tree
Hide file tree
Showing 17 changed files with 266 additions and 45 deletions.
4 changes: 2 additions & 2 deletions crates/rolldown/src/bundler/module_loader/module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ impl<T: FileSystem + 'static + Default> ModuleLoader<T> {
}

fn try_spawn_new_task(&mut self, info: ResolvedRequestInfo) -> ModuleId {
match self.visited.entry(info.path.clone()) {
match self.visited.entry(info.path.path.clone()) {
std::collections::hash_map::Entry::Occupied(visited) => *visited.get(),
std::collections::hash_map::Entry::Vacant(not_visited) => {
let id = Self::alloc_module_id(&mut self.intermediate_modules, &mut self.symbols);
not_visited.insert(id);
if info.is_external {
let ext = ExternalModule::new(id, ResourceId::new(info.path));
let ext = ExternalModule::new(id, ResourceId::new(info.path.path));
self.intermediate_modules[id] = Some(Module::External(ext));
} else {
self.remaining += 1;
Expand Down
28 changes: 15 additions & 13 deletions crates/rolldown/src/bundler/module_loader/normal_module_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use futures::future::join_all;
use index_vec::IndexVec;
use oxc::span::SourceType;
use rolldown_common::{
FilePath, ImportRecordId, ModuleId, ModuleType, RawImportRecord, ResourceId, SymbolRef,
FilePath, ImportRecordId, ModuleId, ModuleType, RawImportRecord, ResolvedPath, ResourceId,
SymbolRef,
};
use rolldown_fs::FileSystem;
use rolldown_oxc::{OxcCompiler, OxcProgram};
Expand Down Expand Up @@ -33,18 +34,18 @@ use crate::{
pub struct NormalModuleTask<'task, T: FileSystem + Default> {
ctx: &'task ModuleTaskCommonData<T>,
module_id: ModuleId,
path: FilePath,
resolved_path: ResolvedPath,
module_type: ModuleType,
}

impl<'task, T: FileSystem + Default + 'static> NormalModuleTask<'task, T> {
pub fn new(
ctx: &'task ModuleTaskCommonData<T>,
id: ModuleId,
path: FilePath,
path: ResolvedPath,
module_type: ModuleType,
) -> Self {
Self { ctx, module_id: id, path, module_type }
Self { ctx, module_id: id, resolved_path: path, module_type }
}
pub async fn run(mut self) {
if let Err(errs) = self.run_inner().await {
Expand All @@ -53,12 +54,12 @@ impl<'task, T: FileSystem + Default + 'static> NormalModuleTask<'task, T> {
}

async fn run_inner(&mut self) -> BatchedResult<()> {
tracing::trace!("process {:?}", self.path);
tracing::trace!("process {:?}", self.resolved_path);

let mut warnings = vec![];

// Run plugin load to get content first, if it is None using read fs as fallback.
let source = load_source(&self.ctx.plugin_driver, &self.path, &self.ctx.fs).await?;
let source = load_source(&self.ctx.plugin_driver, &self.resolved_path, &self.ctx.fs).await?;

// Run plugin transform.
let source = transform_source(&self.ctx.plugin_driver, source).await?;
Expand All @@ -85,7 +86,7 @@ impl<'task, T: FileSystem + Default + 'static> NormalModuleTask<'task, T> {
id: Some(self.module_id),
ast: Some(ast),
repr_name: Some(repr_name),
path: Some(ResourceId::new(self.path.clone())),
path: Some(ResourceId::new(self.resolved_path.path.clone())),
named_imports: Some(named_imports),
named_exports: Some(named_exports),
stmt_infos: Some(stmt_infos),
Expand All @@ -96,7 +97,7 @@ impl<'task, T: FileSystem + Default + 'static> NormalModuleTask<'task, T> {
exports_kind: Some(exports_kind),
namespace_symbol: Some(namespace_symbol),
module_type: self.module_type,
pretty_path: Some(self.path.prettify(&self.ctx.input_options.cwd)),
pretty_path: Some(self.resolved_path.prettify(&self.ctx.input_options.cwd)),
..Default::default()
};

Expand Down Expand Up @@ -139,22 +140,23 @@ impl<'task, T: FileSystem + Default + 'static> NormalModuleTask<'task, T> {
}
let source: Arc<str> = source.into();

let source_type = determine_oxc_source_type(self.path.as_path(), self.module_type);
let source_type =
determine_oxc_source_type(self.resolved_path.path.as_path(), self.module_type);
let program = OxcCompiler::parse(Arc::clone(&source), source_type);

let semantic = program.make_semantic(source_type);
let (mut symbol_table, scope) = semantic.into_symbol_table_and_scope_tree();
let ast_scope = AstScope::new(scope, std::mem::take(&mut symbol_table.references));
let mut symbol_for_module = AstSymbol::from_symbol_table(symbol_table);
let repr_name = self.path.representative_name();
let repr_name = self.resolved_path.path.representative_name();
let scanner = AstScanner::new(
self.module_id,
&ast_scope,
&mut symbol_for_module,
repr_name.into_owned(),
self.module_type,
&source,
&self.path,
&self.resolved_path.path,
);
let namespace_symbol = scanner.namespace_symbol;
let scan_result = scanner.scan(program.program());
Expand Down Expand Up @@ -207,15 +209,15 @@ impl<'task, T: FileSystem + Default + 'static> NormalModuleTask<'task, T> {
// FIXME(hyf0): should not use `Arc<Resolver>` here
let resolver = Arc::clone(&self.ctx.resolver);
let plugin_driver = Arc::clone(&self.ctx.plugin_driver);
let importer = self.path.clone();
let importer = self.resolved_path.clone();
let kind = item.kind;
// let on_warn = self.input_options.on_warn.clone();
tokio::spawn(async move {
Self::resolve_id(
&input_options,
&resolver,
&plugin_driver,
&importer,
&importer.path,
&specifier,
HookResolveIdArgsOptions { is_entry: false, kind },
)
Expand Down
2 changes: 1 addition & 1 deletion crates/rolldown/src/bundler/stages/scan_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ impl<Fs: FileSystem + Default + 'static> ScanStage<Fs> {
{
Ok(info) => {
if info.is_external {
return Err(BuildError::entry_cannot_be_external(info.path.as_str()));
return Err(BuildError::entry_cannot_be_external(info.path.path.as_str()));
}
Ok((input_item.name.clone(), info))
}
Expand Down
17 changes: 10 additions & 7 deletions crates/rolldown/src/bundler/utils/load_source.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
use rolldown_common::FilePath;
use rolldown_common::ResolvedPath;
use sugar_path::AsPath;

use crate::{bundler::plugin_driver::PluginDriver, error::BatchedErrors, HookLoadArgs};

pub async fn load_source(
plugin_driver: &PluginDriver,
path: &FilePath,
resolved_path: &ResolvedPath,
fs: &dyn rolldown_fs::FileSystem,
) -> Result<String, BatchedErrors> {
let source = if let Some(r) = plugin_driver.load(&HookLoadArgs { id: path.as_ref() }).await? {
r.code
} else {
fs.read_to_string(path.as_path())?
};
let source =
if let Some(r) = plugin_driver.load(&HookLoadArgs { id: &resolved_path.path }).await? {
r.code
} else if resolved_path.ignored {
String::new()
} else {
fs.read_to_string(resolved_path.path.as_path())?
};
Ok(source)
}
4 changes: 2 additions & 2 deletions crates/rolldown/src/bundler/utils/resolve_id.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use once_cell::sync::Lazy;
use regex::Regex;
use rolldown_common::{FilePath, ModuleType};
use rolldown_common::{FilePath, ModuleType, ResolvedPath};
use rolldown_error::BuildError;
use rolldown_fs::FileSystem;
use rolldown_resolver::Resolver;
Expand All @@ -16,7 +16,7 @@ static DATA_URL_REGEX: Lazy<Regex> =

#[derive(Debug)]
pub struct ResolvedRequestInfo {
pub path: FilePath,
pub path: ResolvedPath,
pub module_type: ModuleType,
pub is_external: bool,
}
Expand Down
12 changes: 11 additions & 1 deletion crates/rolldown/tests/common/fixture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,17 @@ impl Fixture {
cwd: fixture_path.to_path_buf(),
external: test_config.input.external.map(External::ArrayString).unwrap_or_default(),
treeshake: test_config.input.treeshake.unwrap_or(true),
resolve: None,
resolve: test_config.input.resolve.map(|value| rolldown_resolver::ResolverOptions {
alias: value.alias.map(|alias| alias.into_iter().collect::<Vec<_>>()),
alias_fields: value.alias_fields,
condition_names: value.condition_names,
exports_fields: value.exports_fields,
extensions: value.extensions,
main_fields: value.main_fields,
main_files: value.main_files,
modules: value.modules,
symlinks: value.symlinks,
}),
});

if fixture_path.join("dist").is_dir() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
!node_modules
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
source: crates/rolldown/tests/common/case.rs
expression: content
input_file: crates/rolldown/tests/fixtures/resolve/browser-filed-false
---
# Assets

## package.mjs

```js
import { __commonJS, __toESM } from "./_rolldown_runtime.mjs";
// (ignored) node_modules/package/util.js
// node_modules/package/index.js
import value from './util.js';
console.log(import_util.default);
```

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"input": {
"input": [
{
"name": "package",
"import": "package"
}
],
"resolve": {
"aliasFields": [
[
"browser"
]
]
}
}
}
14 changes: 0 additions & 14 deletions crates/rolldown_common/src/file_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,6 @@ impl FilePath {
name
}

pub fn prettify(&self, cwd: impl AsRef<Path>) -> String {
let pretty = if Path::new(self.0.as_ref()).is_absolute() {
Path::new(self.0.as_ref())
.relative(cwd.as_ref())
.into_os_string()
.into_string()
.expect("should be valid utf8")
} else {
self.0.to_string()
};
// remove \0
pretty.replace('\0', "")
}

pub fn representative_name(&self) -> Cow<str> {
representative_name(&self.0)
}
Expand Down
2 changes: 2 additions & 0 deletions crates/rolldown_common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ mod module_type;
mod named_export;
mod named_import;
mod resolved_export;
mod resolved_path;
mod stmt_info;
mod symbol_ref;
mod wrap_kind;
Expand All @@ -22,6 +23,7 @@ pub use crate::{
named_export::LocalExport,
named_import::{NamedImport, Specifier},
resolved_export::ResolvedExport,
resolved_path::ResolvedPath,
stmt_info::{StmtInfo, StmtInfoId, StmtInfos},
symbol_ref::SymbolRef,
wrap_kind::WrapKind,
Expand Down
41 changes: 41 additions & 0 deletions crates/rolldown_common/src/resolved_path.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
use std::path::Path;

use crate::FilePath;
use sugar_path::SugarPath;

#[derive(Debug, Clone)]
pub struct ResolvedPath {
pub path: FilePath,
pub ignored: bool,
}

impl From<String> for ResolvedPath {
fn from(value: String) -> Self {
Self { path: value.into(), ignored: false }
}
}

impl ResolvedPath {
/// Created a pretty string representation of the path. The path
/// 1. doesn't guarantee to be unique
/// 2. relative to the cwd, so it could show stable path across different machines
pub fn prettify(&self, cwd: impl AsRef<Path>) -> String {
let pretty = if Path::new(self.path.as_ref()).is_absolute() {
Path::new(self.path.as_ref())
.relative(cwd.as_ref())
.into_os_string()
.into_string()
.expect("should be valid utf8")
} else {
self.path.to_string()
};
// remove \0
let pretty = pretty.replace('\0', "");

if self.ignored {
format!("(ignored) {pretty}")
} else {
pretty
}
}
}
21 changes: 16 additions & 5 deletions crates/rolldown_resolver/src/resolver.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use rolldown_common::{FilePath, ModuleType};
use rolldown_common::{FilePath, ModuleType, ResolvedPath};
use rolldown_error::BuildError;
use rolldown_fs::FileSystem;
use std::path::PathBuf;
use sugar_path::{AsPath, SugarPathBuf};

use oxc_resolver::{Resolution, ResolverGeneric};
use oxc_resolver::{Resolution, ResolveError, ResolverGeneric};

use crate::ResolverOptions;

Expand All @@ -28,7 +28,7 @@ impl<F: FileSystem + Default> Resolver<F> {

#[derive(Debug)]
pub struct ResolveRet {
pub resolved: FilePath,
pub resolved: ResolvedPath,
pub module_type: ModuleType,
}

Expand Down Expand Up @@ -66,11 +66,22 @@ impl<F: FileSystem + Default> Resolver<F> {

match resolved {
Ok(info) => Ok(ResolveRet {
resolved: info.path().to_string_lossy().to_string().into(),
resolved: ResolvedPath {
path: info.path().to_string_lossy().to_string().into(),
ignored: false,
},
module_type: calc_module_type(&info),
}),
Err(err) => {
if let Some(importer) = importer {
if let ResolveError::Ignored(path) = err {
Ok(ResolveRet {
resolved: ResolvedPath {
path: path.to_string_lossy().to_string().into(),
ignored: true,
},
module_type: ModuleType::CJS,
})
} else if let Some(importer) = importer {
Err(
BuildError::unresolved_import(specifier.to_string(), importer.as_path())
.with_source(err),
Expand Down
Loading

0 comments on commit c3a82a4

Please sign in to comment.