Skip to content

Commit

Permalink
feat: enable bare import by default (#284)
Browse files Browse the repository at this point in the history
  • Loading branch information
underfin authored Nov 16, 2023
1 parent 083bd3c commit 833dfc6
Show file tree
Hide file tree
Showing 16 changed files with 106 additions and 60 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

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

2 changes: 2 additions & 0 deletions crates/rolldown/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ async-trait = { workspace = true }
smallvec = { workspace = true }
oxc_resolver = { workspace = true }
miette = { workspace = true }
regex = { workspace = true }
once_cell = { workspace = true }

[dev_dependencies]
insta = { workspace = true }
Expand Down
32 changes: 8 additions & 24 deletions crates/rolldown/src/bundler/module_loader/normal_module_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,33 +191,17 @@ impl<'task, T: FileSystem + Default + 'static> NormalModuleTask<'task, T> {
});
}

let resolved_id =
let mut info =
resolve_id(resolver, plugin_driver, specifier, Some(importer), options, false).await?;

match resolved_id {
Some(mut info) => {
if !info.is_external {
// Check external with resolved path
info.is_external = input_options
.external
.call(specifier.to_string(), Some(importer.to_string()), true)
.await?;
}
Ok(info)
}
None => {
Ok(ResolvedRequestInfo {
path: specifier.to_string().into(),
module_type: ModuleType::Unknown,
is_external: true,
})
// // TODO: should emit warnings like https://rollupjs.org/guide/en#warning-treating-module-as-external-dependency
// return Err(rolldown_error::Error::unresolved_import(
// specifier.to_string(),
// importer.prettify(),
// ));
}
if !info.is_external {
// Check external with resolved path
info.is_external = input_options
.external
.call(specifier.to_string(), Some(importer.to_string()), true)
.await?;
}
Ok(info)
}

async fn resolve_dependencies(
Expand Down
7 changes: 1 addition & 6 deletions crates/rolldown/src/bundler/stages/scan_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,10 @@ impl<Fs: FileSystem + Default + 'static> ScanStage<Fs> {
)
.await
{
Ok(r) => {
let Some(info) = r else {
return Err(BuildError::unresolved_entry(specifier));
};

Ok(info) => {
if info.is_external {
return Err(BuildError::entry_cannot_be_external(info.path.as_str()));
}

Ok((input_item.name.clone(), info))
}
Err(e) => Err(e),
Expand Down
41 changes: 27 additions & 14 deletions crates/rolldown/src/bundler/utils/resolve_id.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use once_cell::sync::Lazy;
use regex::Regex;
use rolldown_common::{FilePath, ModuleType};
use rolldown_error::BuildError;
use rolldown_fs::FileSystem;
Expand All @@ -7,6 +9,11 @@ use crate::{
bundler::plugin_driver::SharedPluginDriver, HookResolveIdArgs, HookResolveIdArgsOptions,
};

static HTTP_URL_REGEX: Lazy<Regex> =
Lazy::new(|| Regex::new(r"^(https?:)?\/\/").expect("Init HTTP_URL_REGEX failed"));
static DATA_URL_REGEX: Lazy<Regex> =
Lazy::new(|| Regex::new(r"^\s*data:").expect("Init DATA_URL_REGEX failed"));

#[derive(Debug)]
pub struct ResolvedRequestInfo {
pub path: FilePath,
Expand All @@ -22,7 +29,7 @@ pub async fn resolve_id<T: FileSystem + Default>(
importer: Option<&FilePath>,
options: HookResolveIdArgsOptions,
_preserve_symlinks: bool,
) -> Result<Option<ResolvedRequestInfo>, BuildError> {
) -> Result<ResolvedRequestInfo, BuildError> {
// Run plugin resolve_id first, if it is None use internal resolver as fallback
if let Some(r) = plugin_driver
.resolve_id(&HookResolveIdArgs {
Expand All @@ -32,23 +39,29 @@ pub async fn resolve_id<T: FileSystem + Default>(
})
.await?
{
return Ok(Some(ResolvedRequestInfo {
return Ok(ResolvedRequestInfo {
path: r.id.into(),
module_type: ModuleType::Unknown,
is_external: matches!(r.external, Some(true)),
}));
});
}

// external modules (non-entry modules that start with neither '.' or '/')
// are skipped at this stage.
if importer.is_some() && !request.starts_with('.') {
Ok(None)
} else {
let resolved = resolver.resolve(importer, request)?;
Ok(Some(ResolvedRequestInfo {
path: resolved.resolved,
module_type: resolved.module_type,
is_external: false,
}))
// Auto external http url or data url
if HTTP_URL_REGEX.is_match(request) || DATA_URL_REGEX.is_match(request) {
return Ok(ResolvedRequestInfo {
path: request.to_string().into(),
module_type: ModuleType::Unknown,
is_external: true,
});
}

// Rollup external node packages by default.
// Rolldown will follow esbuild behavior to resolve it by default.
// See https://github.com/rolldown-rs/rolldown/issues/282
let resolved = resolver.resolve(importer, request)?;
Ok(ResolvedRequestInfo {
path: resolved.resolved,
module_type: resolved.module_type,
is_external: false,
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
"name": "entry_js",
"import": "entry.js"
}
],
"external": [
"supports-color"
]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
"name": "entry_js",
"import": "entry.js"
}
],
"external": [
"foo"
]
}
}
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
{}
{
"input": {
"external": [
"assert"
]
}
}
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
{}
{
"input": {
"external": [
"assert"
]
}
}
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
{}
{
"input": {
"external": [
"assert"
]
}
}
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
{}
{
"input": {
"external": [
"assert"
]
}
}
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
{}
{
"input": {
"external": [
"assert"
]
}
}
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
{}
{
"input": {
"external": [
"node:assert"
]
}
}
Original file line number Diff line number Diff line change
@@ -1 +1,8 @@
{}
{
"input": {
"external": [
"ext1",
"ext2"
]
}
}
Original file line number Diff line number Diff line change
@@ -1 +1,8 @@
{}
{
"input": {
"external": [
"node:module",
"node:assert"
]
}
}
10 changes: 2 additions & 8 deletions crates/rolldown_plugin_vite_scanner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@ use rolldown_fs::FileSystem;
use std::{borrow::Cow, fmt::Debug, path::PathBuf};
use util::{extract_html_module_scripts, VIRTUAL_MODULE_PREFIX};
mod util;
static HTTP_URL_REGEX: Lazy<Regex> =
Lazy::new(|| Regex::new(r"^(https?:)?\/\/").expect("Init HTTP_URL_REGEX failed"));
static DATA_URL_REGEX: Lazy<Regex> =
Lazy::new(|| Regex::new(r"^\s*data:").expect("Init DATA_URL_REGEX failed"));

static VIRTUAL_MODULE_REGEX: Lazy<Regex> =
Lazy::new(|| Regex::new(r#"^virtual-module:.*"#).expect("Init VIRTUAL_MODULE_REGEX failed"));
static VITE_SPECIAL_QUERY_REGEX: Lazy<Regex> = Lazy::new(|| {
Expand Down Expand Up @@ -63,10 +60,7 @@ impl<T: FileSystem + 'static + Default> Plugin for ViteScannerPlugin<T> {
) -> HookResolveIdReturn {
let HookResolveIdArgs { source, .. } = args;

// External http url or data url
if HTTP_URL_REGEX.is_match(source) || DATA_URL_REGEX.is_match(source) {
return Ok(Some(HookResolveIdOutput { id: (*source).to_string(), external: Some(true) }));
}
// http url or data url is already external at rolldown resolver

// resolve local scripts (`<script>` in Svelte and `<script setup>` in Vue)
if VIRTUAL_MODULE_REGEX.is_match(source) {
Expand Down

0 comments on commit 833dfc6

Please sign in to comment.