Skip to content

Commit

Permalink
Make side effect import sorting deterministic
Browse files Browse the repository at this point in the history
This fixes the test failures on Node.js 10 and 8.
  • Loading branch information
lydell committed Nov 22, 2019
1 parent 06c4db7 commit d1d59be
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 8 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -364,8 +364,8 @@ The inner arrays are joined with one newline; the outer arrays are joined with
two (creating a blank line).

Every group is sorted internally as mentioned in [Sort order]. Side effect
imports are sorted too – but will keep their internal order. It’s recommended to
keep side effect imports in their own group.
imports are always placed first in the group and keep their internal order. It’s
recommended to keep side effect imports in their own group.

These are the default groups:

Expand Down
2 changes: 1 addition & 1 deletion examples/groups.custom.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {CATEGORIES} from "../"
import {truncate, removeWhitespace} from "../../utils";
import cssGlobals from "../../css/globals"
import styles from "./styles.scss";
import circleSyles from "./circle.scss";
import circleStyles from "./circle.scss";
import "./global.scss"
import {name} from "@company/config"
import Button from "@ui/Button";
Expand Down
7 changes: 6 additions & 1 deletion src/sort.js
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,12 @@ function sortImportItems(items) {
// If both items are side effect imports, keep their original order.
itemA.isSideEffectImport && itemB.isSideEffectImport
? 0
: // First compare the `from` part.
: // If one of the items is a side effect import, move it first.
itemA.isSideEffectImport
? -1
: itemB.isSideEffectImport
? 1
: // Compare the `from` part.
compare(itemA.source.source, itemB.source.source) ||
// The `.source` has been slightly tweaked. To stay fully deterministic,
// also sort on the original value.
Expand Down
2 changes: 1 addition & 1 deletion test/__snapshots__/examples.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,9 @@ import tomorrow from "./time/tomorrow"
import {providers} from "./providers"
import {PRODUCT_NAMES} from "."
import circleSyles from "./circle.scss";
import "./global.scss"
import "../../alert.css"
import circleStyles from "./circle.scss";
import styles from "./styles.scss";
`;
Expand Down
16 changes: 13 additions & 3 deletions test/sort.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,15 @@ function getLoc(depth = 1) {
return match != null ? match[0] : "?";
}

function ifSupported(regexString, fallbackRegexString) {
try {
RegExp(regexString, "u");
return regexString;
} catch (_error) {
return fallbackRegexString;
}
}

const baseTests = expect => ({
valid: [
// Simple cases.
Expand Down Expand Up @@ -1433,8 +1442,9 @@ const baseTests = expect => ({
},

// `groups` – `u` flag.
// Node.js 8 supports `u` but not `\p{L}`.
{
options: [{ groups: [["^\\p{L}"], ["^\\."]] }],
options: [{ groups: [[ifSupported("^\\p{L}", "^[^.]")], ["^\\."]] }],
code: input`
|import b from '.';
|import a from 'ä';
Expand Down Expand Up @@ -1533,11 +1543,11 @@ const baseTests = expect => ({
`,
output: actual => {
expect(actual).toMatchInlineSnapshot(`
|import x from './x';
|import b from 'b';
|import 'c';
|import 'a';
|import '.';
|import x from './x';
|import b from 'b';
|import d from 'd';
`);
},
Expand Down

0 comments on commit d1d59be

Please sign in to comment.