Skip to content

Commit

Permalink
fix: make outdir behavior compatible with rollup if file is used (rol…
Browse files Browse the repository at this point in the history
…ldown#3304)

<!-- Thank you for contributing! -->

### Description
close rolldown#3240
<!-- Please insert your description here and provide especially info
about the "what" this PR is solving -->
  • Loading branch information
underfin authored Jan 7, 2025
1 parent 7aeb64c commit a1fb0ff
Show file tree
Hide file tree
Showing 20 changed files with 66 additions and 47 deletions.
2 changes: 1 addition & 1 deletion crates/rolldown/src/asset/asset_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ impl Generator for AssetGenerator {
let preliminary_filename =
ctx.chunk.asset_preliminary_filenames.get(&asset_module.idx).unpack();
let file_path =
ctx.options.cwd.as_path().join(&ctx.options.dir).join(preliminary_filename.as_str());
ctx.options.cwd.as_path().join(&ctx.options.out_dir).join(preliminary_filename.as_str());
let file_dir = file_path.parent().expect("chunk file name should have a parent");
instantiated_chunks.push(InstantiatedChunk {
origin_chunk: ctx.chunk_idx,
Expand Down
8 changes: 2 additions & 6 deletions crates/rolldown/src/bundler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rolldown_fs::{FileSystem, OsFileSystem};
use rolldown_plugin::{
HookBuildEndArgs, HookRenderErrorArgs, SharedPluginDriver, __inner::SharedPluginable,
};
use std::{borrow::Cow, sync::Arc};
use std::sync::Arc;
use tracing_chrome::FlushGuard;

pub struct Bundler {
Expand Down Expand Up @@ -99,11 +99,7 @@ impl Bundler {
) -> BuildResult<BundleOutput> {
let mut output = self.bundle_up(scan_stage_output, /* is_write */ true).await?;

let dist_dir = if self.options.file.is_some() {
Cow::Borrowed(&self.options.cwd)
} else {
Cow::Owned(self.options.cwd.join(&self.options.dir))
};
let dist_dir = self.options.cwd.join(&self.options.out_dir);

self.fs.create_dir_all(&dist_dir).map_err(|err| {
anyhow::anyhow!("Could not create directory for output chunks: {:?}", dist_dir).context(err)
Expand Down
2 changes: 1 addition & 1 deletion crates/rolldown/src/css/css_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl Generator for CssGenerator {

// Here file path is generated by chunk file name template, it maybe including path segments.
// So here need to read it's parent directory as file_dir.
let file_path = ctx.options.cwd.as_path().join(&ctx.options.dir).join(
let file_path = ctx.options.cwd.as_path().join(&ctx.options.out_dir).join(
ctx
.chunk
.css_preliminary_filename
Expand Down
2 changes: 1 addition & 1 deletion crates/rolldown/src/ecmascript/ecma_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ impl Generator for EcmaGenerator {

// Here file path is generated by chunk file name template, it maybe including path segments.
// So here need to read it's parent directory as file_dir.
let file_path = ctx.options.cwd.as_path().join(&ctx.options.dir).join(
let file_path = ctx.options.cwd.as_path().join(&ctx.options.out_dir).join(
ctx
.chunk
.preliminary_filename
Expand Down
6 changes: 3 additions & 3 deletions crates/rolldown/src/stages/generate_stage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ impl<'a> GenerateStage<'a> {
chunk.asset_absolute_preliminary_filenames.insert(
module.idx,
preliminary
.absolutize_with(self.options.cwd.join(&self.options.dir))
.absolutize_with(self.options.cwd.join(&self.options.out_dir))
.expect_into_string(),
);
chunk.asset_preliminary_filenames.insert(module.idx, preliminary);
Expand All @@ -282,12 +282,12 @@ impl<'a> GenerateStage<'a> {

chunk.absolute_preliminary_filename = Some(
preliminary_filename
.absolutize_with(self.options.cwd.join(&self.options.dir))
.absolutize_with(self.options.cwd.join(&self.options.out_dir))
.expect_into_string(),
);
chunk.css_absolute_preliminary_filename = Some(
css_preliminary_filename
.absolutize_with(self.options.cwd.join(&self.options.dir))
.absolutize_with(self.options.cwd.join(&self.options.out_dir))
.expect_into_string(),
);
chunk.preliminary_filename = Some(preliminary_filename);
Expand Down
17 changes: 16 additions & 1 deletion crates/rolldown/src/utils/normalize_options.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::path::Path;

use oxc::transformer::InjectGlobalVariablesConfig;
use rolldown_common::{
Comments, GlobalsOutputOption, InjectImport, ModuleType, NormalizedBundlerOptions, OutputFormat,
Expand Down Expand Up @@ -142,6 +144,18 @@ pub fn normalize_options(mut raw_options: crate::BundlerOptions) -> NormalizeOpt
_ => raw_options.inline_dynamic_imports.unwrap_or(false),
};

// If the `file` is provided, use the parent directory of the file as the `out_dir`.
// Otherwise, use the `dir` if provided, or default to `dist`.
let out_dir = raw_options.file.as_ref().map_or_else(
|| raw_options.dir.clone().unwrap_or_else(|| "dist".to_string()),
|file| {
Path::new(file.as_str())
.parent()
.map(|parent| parent.to_string_lossy().to_string())
.unwrap_or_default()
},
);

let normalized = NormalizedBundlerOptions {
input: raw_options.input.unwrap_or_default(),
cwd: raw_options
Expand Down Expand Up @@ -170,7 +184,8 @@ pub fn normalize_options(mut raw_options: crate::BundlerOptions) -> NormalizeOpt
intro: raw_options.intro,
outro: raw_options.outro,
es_module: raw_options.es_module.unwrap_or_default(),
dir: raw_options.dir.unwrap_or_else(|| "dist".to_string()),
dir: raw_options.dir,
out_dir,
file: raw_options.file,
format,
exports: raw_options.exports.unwrap_or(crate::OutputExports::Auto),
Expand Down
3 changes: 2 additions & 1 deletion crates/rolldown/src/watch/watcher_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ impl WatcherTask {
match result {
Ok(()) => {
self.emitter.emit(WatcherEvent::Event(BundleEvent::BundleEnd(BundleEndEventData {
output: bundler.options.cwd.join(&bundler.options.dir).to_string_lossy().to_string(),
// TODO Here should be the file if specified `file` option
output: bundler.options.cwd.join(&bundler.options.out_dir).to_string_lossy().to_string(),
#[allow(clippy::cast_possible_truncation)]
duration: start_time.elapsed().as_millis() as u32,
})))?;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"config": {
"file": "dist.js",
"file": "dist/index.js",
"name": "wrap"
},
"expectError": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ snapshot_kind: text
## dist/out.css

```css
body {
background-color: #f0f0f0;
}
```
## dist/out.css
## out.css

```css
body {
background-color: #f0f0f0;
}
```
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"config": {
"file": "dist/out.css"
"file": "dist/out.js"
},
"expectExecuted": false
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ snapshot_kind: text
---
# Assets

## dist/out.css
## out.js

```css
```js
//#region main.js
var main_default = "hello, world";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ snapshot_kind: text

## dist/out.css

```css
body {
background-color: #f0f0f0;
}
```
## out.css

```css
//#region main.js
Expand All @@ -14,11 +22,3 @@ var main_default = "hello, world";
//#endregion
export { main_default as default };
```
## dist/out.css

```css
body {
background-color: #f0f0f0;
}
```
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ snapshot_kind: text
```
# Assets

## dist/main.js
## main.js

```js
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4103,16 +4103,16 @@ snapshot_kind: text
# tests/rolldown/function/file/css
- dist/out.css => dist/out.css
- out.css => out.css
- dist/out.css
# tests/rolldown/function/file/js
- dist/out.css => dist/out.css
- out.js => out.js
# tests/rolldown/function/file/js_css
- dist/out.css => dist/out.css
- out.css => out.css
- dist/out.css
# tests/rolldown/function/format/app/export-all
Expand Down Expand Up @@ -5296,7 +5296,7 @@ snapshot_kind: text
# tests/rolldown/warnings/invalid_option/invalid_output_dir_option
- dist/main.js => dist/main.js
- main.js => main.js
# tests/rolldown/warnings/invalid_option/unsupported_code_splitting_format
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,7 @@ impl BindingNormalizedOptions {

#[napi(getter)]
pub fn dir(&self) -> Option<String> {
// NOTE: rollup returns undefined when `dir` is not set
Some(self.inner.dir.clone())
self.inner.dir.clone()
}

#[napi(getter)]
Expand Down
7 changes: 6 additions & 1 deletion crates/rolldown_common/src/chunk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,12 @@ impl Chunk {
make_unique_name: &mut impl FnMut(&ArcStr) -> ArcStr,
) -> anyhow::Result<PreliminaryFilename> {
if let Some(file) = &options.file {
return Ok(PreliminaryFilename::new(file.clone(), None));
let basename = PathBuf::from(file)
.file_name()
.expect("The file should have basename")
.to_string_lossy()
.to_string();
return Ok(PreliminaryFilename::new(basename, None));
}
let filename_template = self.filename_template(options, rollup_pre_rendered_chunk).await?;
let extracted_hash_pattern = extract_hash_pattern(filename_template.template());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ pub struct NormalizedBundlerOptions {
pub entry_filenames: ChunkFilenamesOutputOption,
pub chunk_filenames: ChunkFilenamesOutputOption,
pub asset_filenames: FilenameTemplate,
pub dir: String,
// The user specified output directory config
pub dir: Option<String>,
// The rolldown resolved output directory from `dir` or `file`.
pub out_dir: String,
pub file: Option<String>,
pub format: OutputFormat,
pub exports: OutputExports,
Expand Down
12 changes: 6 additions & 6 deletions crates/rolldown_testing/src/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ impl IntegrationTest {
let mut bundler = Bundler::new(options);

if self.test_meta.write_to_disk {
if bundler.options().dir.as_path().is_dir() {
std::fs::remove_dir_all(&bundler.options().dir)
.context(bundler.options().dir.clone())
if bundler.options().out_dir.as_path().is_dir() {
std::fs::remove_dir_all(&bundler.options().out_dir)
.context(bundler.options().out_dir.clone())
.expect("Failed to clean the output directory");
}
bundler.write().await
Expand All @@ -73,7 +73,7 @@ impl IntegrationTest {
let cwd = bundler.options().cwd.clone();

let bundle_output = if self.test_meta.write_to_disk {
let abs_output_dir = cwd.join(&bundler.options().dir);
let abs_output_dir = cwd.join(&bundler.options().out_dir);
if abs_output_dir.is_dir() {
std::fs::remove_dir_all(&abs_output_dir)
.context(format!("{abs_output_dir:?}"))
Expand Down Expand Up @@ -128,7 +128,7 @@ impl IntegrationTest {
let cwd = bundler.options().cwd.clone();

let bundle_output = if self.test_meta.write_to_disk {
let abs_output_dir = cwd.join(&bundler.options().dir);
let abs_output_dir = cwd.join(&bundler.options().out_dir);
if abs_output_dir.is_dir() {
std::fs::remove_dir_all(&abs_output_dir)
.context(format!("{abs_output_dir:?}"))
Expand Down Expand Up @@ -440,7 +440,7 @@ impl IntegrationTest {

fn execute_output_assets(bundler: &Bundler, test_title: &str) {
let cwd = bundler.options().cwd.clone();
let dist_folder = cwd.join(&bundler.options().dir);
let dist_folder = cwd.join(&bundler.options().out_dir);

let is_expect_executed_under_esm = matches!(bundler.options().format, OutputFormat::Esm)
|| (!matches!(bundler.options().format, OutputFormat::Cjs)
Expand Down
2 changes: 1 addition & 1 deletion packages/rolldown/tests/fixtures/output/file/_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ export default defineTest({
},
},
afterTest: function (output) {
expect(output.output[0].fileName).toMatchInlineSnapshot(`"dist/out.js"`)
expect(output.output[0].fileName).toBe('out.js')
},
})
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export default defineTest({
expect(option.chunkFileNames).toBeInstanceOf(Function)
expect(option.assetFileNames).toBe('assets/[name]-[hash][extname]')
expect(option.file).toBe('dist/[name].js')
expect(option.dir).toBe('dist')
expect(option.dir).toBe(undefined)
expect(option.format).toBe('umd')
expect(option.exports).toBe('auto')
expect(option.esModule).toBe('if-default-prop')
Expand Down

0 comments on commit a1fb0ff

Please sign in to comment.