Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
fix(rome_js_formatter): Trailing comma inside import #3600 (#3624)
Browse files Browse the repository at this point in the history 
Fix #3600
  • Loading branch information
@denbezrukov
denbezrukov committed Nov 9, 2022
1 parent 76a56db commit b788d70
Show file tree
Hide file tree
Showing 7 changed files with 145 additions and 115 deletions.
21 changes: 16 additions & 5 deletions crates/rome_js_formatter/src/js/expressions/call_arguments.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ use rome_formatter::{format_args, format_element, write, VecBuffer};
use rome_js_syntax::{
JsAnyCallArgument, JsAnyExpression, JsAnyFunctionBody, JsAnyLiteralExpression, JsAnyStatement,
JsCallArgumentList, JsCallArguments, JsCallArgumentsFields, JsCallExpression,
JsExpressionStatement, JsFunctionExpression, JsLanguage, TsAnyReturnType, TsType,
JsExpressionStatement, JsFunctionExpression, JsImportCallExpression, JsLanguage,
TsAnyReturnType, TsType,
};
use rome_rowan::{AstSeparatedElement, AstSeparatedList, SyntaxResult};

Expand Down Expand Up @@ -99,6 +100,7 @@ impl FormatNodeRule<JsCallArguments> for FormatJsCallArguments {
l_paren: &l_paren_token.format(),
args: &arguments,
r_paren: &r_paren_token.format(),
node,
expand: true,
}]
);
Expand All @@ -124,7 +126,8 @@ impl FormatNodeRule<JsCallArguments> for FormatJsCallArguments {
l_paren: &l_paren_token.format(),
args: &arguments,
r_paren: &r_paren_token.format(),
expand: false
node,
expand: false,
}]
)
}
Expand Down Expand Up @@ -344,7 +347,8 @@ fn write_grouped_arguments(
l_paren: &l_paren_token.format(),
args: &arguments,
r_paren: &r_paren_token.format(),
expand: true
node: call_arguments,
expand: true,
}]
);
}
Expand Down Expand Up @@ -385,7 +389,8 @@ fn write_grouped_arguments(
l_paren: &l_paren,
args: &arguments,
r_paren: &r_paren,
expand: true
node: call_arguments,
expand: true,
}]
)?;
buffer.write_element(FormatElement::Tag(Tag::EndEntry))?;
Expand Down Expand Up @@ -703,10 +708,13 @@ struct FormatAllArgsBrokenOut<'a> {
args: &'a [FormatCallArgument],
r_paren: &'a dyn Format<JsFormatContext>,
expand: bool,
node: &'a JsCallArguments,
}

impl<'a> Format<JsFormatContext> for FormatAllArgsBrokenOut<'a> {
fn fmt(&self, f: &mut Formatter<JsFormatContext>) -> FormatResult<()> {
let is_inside_import = self.node.parent::<JsImportCallExpression>().is_some();

write!(
f,
[group(&format_args![
Expand All @@ -723,7 +731,10 @@ impl<'a> Format<JsFormatContext> for FormatAllArgsBrokenOut<'a> {
write!(f, [entry])?;
}

write!(f, [FormatTrailingComma::All])
if !is_inside_import {
write!(f, [FormatTrailingComma::All])?;
}
Ok(())
})),
self.r_paren,
])
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_js_formatter/src/js/expressions/import_call_expression.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ impl FormatNodeRule<JsImportCallExpression> for FormatJsImportCallExpression {
arguments,
} = node.as_fields();

write![f, [import_token.format(), arguments.format(),]]
write![f, [import_token.format(), arguments.format()]]
}

fn needs_parentheses(&self, item: &JsImportCallExpression) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_js_formatter/tests/specs/js/module/import/import_call.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import(x);
import("x");
import(
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
{ assert: { type: "json" } },
{ assert: { type: "json" } }
);


Expand Down
20 changes: 20 additions & 0 deletions ...es/rome_js_formatter/tests/specs/js/module/import/trailing-comma/import_trailing_comma.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,23 @@ import {
adsadasdasdasdasdasdasdasdasdasdas,
dsadsadasdasdasdasdasdasdasd,
dsadsadasdasdasdasdasdasdasd, } from "W";

const StandaloneBackendWASMModule = await import(/* webpackChunkName: "standalone" */'./StandaloneBackendWASM');

const StandaloneBackendWASMModule = await import(/* webpackChunkName: "standalone" */'./StandaloneBackendWASM',
/* webpackChunkName: "standalone" */'./StandaloneBackendWASM');

import(
'myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename'
);

import(/* Hello */ "something");
import("something" /* Hello */);
import(/* Hello */ "something" /* Hello */);
import("something" /* Hello */ + "else");
import(
/* Hello */
"something",
/* Hello */
);
wrap(import(/* Hello */ "something"));
107 changes: 107 additions & 0 deletions ...me_js_formatter/tests/specs/js/module/import/trailing-comma/import_trailing_comma.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,26 @@ import {
dsadsadasdasdasdasdasdasdasd,
dsadsadasdasdasdasdasdasdasd, } from "W";

const StandaloneBackendWASMModule = await import(/* webpackChunkName: "standalone" */'./StandaloneBackendWASM');

const StandaloneBackendWASMModule = await import(/* webpackChunkName: "standalone" */'./StandaloneBackendWASM',
/* webpackChunkName: "standalone" */'./StandaloneBackendWASM');

import(
'myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename'
);

import(/* Hello */ "something");
import("something" /* Hello */);
import(/* Hello */ "something" /* Hello */);
import("something" /* Hello */ + "else");
import(
/* Hello */
"something",
/* Hello */
);
wrap(import(/* Hello */ "something"));

```


Expand Down Expand Up @@ -46,6 +66,35 @@ import {
dsadsadasdasdasdasdasdasdasd,
dsadsadasdasdasdasdasdasdasd,
} from "W";

const StandaloneBackendWASMModule = await import(
/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM"
);

const StandaloneBackendWASMModule = await import(
/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM",
/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM"
);

import(
"myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename"
);

import(/* Hello */ "something");
import("something" /* Hello */);
import(/* Hello */ "something" /* Hello */);
import("something" /* Hello */ + "else");
import(
/* Hello */
"something"
/* Hello */
);
wrap(import(/* Hello */ "something"));


## Lines exceeding width of 80 characters

23: "myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename"
```

## Output 2
Expand All @@ -70,6 +119,35 @@ import {
dsadsadasdasdasdasdasdasdasd,
dsadsadasdasdasdasdasdasdasd,
} from "W";

const StandaloneBackendWASMModule = await import(
/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM"
);

const StandaloneBackendWASMModule = await import(
/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM",
/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM"
);

import(
"myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename"
);

import(/* Hello */ "something");
import("something" /* Hello */);
import(/* Hello */ "something" /* Hello */);
import("something" /* Hello */ + "else");
import(
/* Hello */
"something"
/* Hello */
);
wrap(import(/* Hello */ "something"));


## Lines exceeding width of 80 characters

23: "myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename"
```

## Output 3
Expand All @@ -94,6 +172,35 @@ import {
dsadsadasdasdasdasdasdasdasd,
dsadsadasdasdasdasdasdasdasd
} from "W";

const StandaloneBackendWASMModule = await import(
/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM"
);

const StandaloneBackendWASMModule = await import(
/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM",
/* webpackChunkName: "standalone" */ "./StandaloneBackendWASM"
);

import(
"myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename"
);

import(/* Hello */ "something");
import("something" /* Hello */);
import(/* Hello */ "something" /* Hello */);
import("something" /* Hello */ + "else");
import(
/* Hello */
"something"
/* Hello */
);
wrap(import(/* Hello */ "something"));


## Lines exceeding width of 80 characters

23: "myreallylongdynamicallyloadedmodulenamemyreallylongdynamicallyloadedmodulename"
```


69 changes: 0 additions & 69 deletions crates/rome_js_formatter/tests/specs/prettier/js/comments/dynamic_imports.js.snap
View file

This file was deleted.

39 changes: 0 additions & 39 deletions crates/rome_js_formatter/tests/specs/prettier/js/trailing-comma/dynamic-import.js.snap
View file

This file was deleted.

0 comments on commit b788d70

Please sign in to comment.