Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Add <BUILTIN_MODULES> Special Word #86

Merged
merged 3 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Since then more critical features & fixes have been added, and the options have

[We welcome contributions!](./CONTRIBUTING.md)

**Table of Contents**
## Table of Contents

- [Sample](#sample)
- [Input](#input)
Expand Down Expand Up @@ -177,9 +177,10 @@ unsortable. This can be used for edge-cases, such as when you have a named impor

Next, the plugin sorts the _local imports_ and _third party imports_ using [natural sort algorithm](https://en.wikipedia.org/wiki/Natural_sort_order).

In the end, the plugin returns final imports with _third party imports_ on top and _local imports_ at the end.
In the end, the plugin returns final imports with _nodejs built-in modules_, followed by _third party imports_ and subsequent _local imports_ at the end.

The _third party imports_ position (it's top by default) can be overridden using the `<THIRD_PARTY_MODULES>` special word in the `importOrder`.
- The _nodejs built-in modules_ position (it's 1st by default) can be overridden using the `<BUILTIN_MODULES>` special word in the `importOrder`
- The _third party imports_ position (it's 2nd by default) can be overridden using the `<THIRD_PARTY_MODULES>` special word in the `importOrder`.

### Options

Expand All @@ -193,14 +194,20 @@ The main way to control the import order and formatting, `importOrder` is a coll

```js
[
// node.js built-ins are always first
'<THIRD_PARTY_MODULES>', // Non-relative imports
'<BUILTIN_MODULES>', // Node.js built-in modules
'<THIRD_PARTY_MODULES>', // Imports not matched by other special words or groups.
'^[.]', // relative imports
],
```

By default, this plugin sorts as documented on the line above, with Node.js built-in modules at the top, followed by non-relative imports, and lastly any relative import starting with a `.` character.

Available Special Words:

- `<BUILTIN_MODULES>` - All _nodejs built-in modules_ will be grouped here, and is injected at the top if it's not present.
- `<THIRD_PARTY_MODULES>` - All imports not targeted by another regex will end up here, so this will be injected if not present in `options.importOrder`
- `<TYPES>` - Not active by default, this allows you to group all type-imports, or target them with a regex (`<TYPES>^[.]` targets imports of types from local files).

Here are some common ways to configure `importOrder`:

##### 1. Put specific dependencies at the top
Expand Down Expand Up @@ -240,11 +247,9 @@ import styles from './global.css';
If you want to group your imports into "chunks" with blank lines between, you can add empty strings like this:

```json
"importOrder": ["", "<THIRD_PARTY_MODULES>", "", "^[.]",]
"importOrder": ["<BUILT_IN_MODULES>", "", "<THIRD_PARTY_MODULES>", "", "^[.]"]
```

(Note the empty string at the start, to add a blank line after node.js built-ins)

e.g.:

```ts
Expand Down
11 changes: 7 additions & 4 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,14 @@ export const mergeableImportFlavors = [
importFlavorType,
] as const;

/*
* Used to mark the position between RegExps,
* where the not matched imports should be placed
export const BUILTIN_MODULES_REGEX_STR = `^(?:node:)?(?:${builtinModules.join(
'|',
)})$`;

export const BUILTIN_MODULES_SPECIAL_WORD = '<BUILTIN_MODULES>';
/**
* Used to mark not otherwise matched imports should be placed
*/
export const BUILTIN_MODULES = `^(?:node:)?(?:${builtinModules.join('|')})$`;
export const THIRD_PARTY_MODULES_SPECIAL_WORD = '<THIRD_PARTY_MODULES>';
export const TYPES_SPECIAL_WORD = '<TYPES>';

Expand Down
7 changes: 5 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import { parsers as flowParsers } from 'prettier/parser-flow';
import { parsers as htmlParsers } from 'prettier/parser-html';
import { parsers as typescriptParsers } from 'prettier/parser-typescript';

import { THIRD_PARTY_MODULES_SPECIAL_WORD } from './constants';
import {
BUILTIN_MODULES_SPECIAL_WORD,
THIRD_PARTY_MODULES_SPECIAL_WORD,
} from './constants';
import { defaultPreprocessor } from './preprocessors/default';
import { vuePreprocessor } from './preprocessors/vue';
import type { PrettierOptions } from './types';
Expand All @@ -29,7 +32,7 @@ export const options: Record<
default: [
{
value: [
// node.js built-ins are always first
BUILTIN_MODULES_SPECIAL_WORD,
THIRD_PARTY_MODULES_SPECIAL_WORD, // Everything not matching relative imports
'^[.]', // relative imports
],
Expand Down
1 change: 0 additions & 1 deletion src/utils/get-comment-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
type ImportDeclaration,
} from '@babel/types';

import { newLineNode } from '../constants';
import { ImportOrLine, ImportRelated, SomeSpecifier } from '../types';

const SpecifierTypes = [
Expand Down
16 changes: 13 additions & 3 deletions src/utils/get-import-nodes-matched-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,24 @@ import {
TYPES_SPECIAL_WORD,
} from '../constants';

const regexCache = new Map<string, RegExp>();
const cachedRegExp = (regExp: string) => {
if (regexCache.has(regExp)) {
return regexCache.get(regExp)!;
}
// Strip <TYPES> when creating regexp
const result = new RegExp(regExp.replace(TYPES_SPECIAL_WORD, ''));
regexCache.set(regExp, result);
return result;
};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice little perf improvement here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted also to parse importOrder and normalize it only once, but that requires a further cleanup because there’s logic that looks at the first entry in importOrder and does something if it’s a space.

I decided to submit this PR as-is and consider if it’s worth changing that at some future date.


/**
* Get the regexp group to keep the import nodes.
*
* This comes near the end of processing, after import declaration nodes have been combined or exploded.
*
* @param node
* @param importOrder
* @param importOrder a list of [regexp or special-word] groups (no separators)
*/
export const getImportNodesMatchedGroup = (
node: ImportDeclaration,
Expand All @@ -23,8 +34,7 @@ export const getImportNodesMatchedGroup = (
const groupWithRegExp = importOrder
.map((group) => ({
group,
// Strip <TYPES> when creating regexp
regExp: new RegExp(group.replace(TYPES_SPECIAL_WORD, '')),
regExp: cachedRegExp(group),
}))
// Remove explicit bare <TYPES> group, we'll deal with that at the end similar to third party modules
.filter(({ group }) => group !== TYPES_SPECIAL_WORD);
Expand Down
17 changes: 4 additions & 13 deletions src/utils/get-sorted-nodes-by-import-order.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import {
BUILTIN_MODULES,
newLineNode,
THIRD_PARTY_MODULES_SPECIAL_WORD,
} from '../constants';
import { newLineNode, THIRD_PARTY_MODULES_SPECIAL_WORD } from '../constants';
import type { GetSortedNodes, ImportGroups, ImportOrLine } from '../types';
import { getImportNodesMatchedGroup } from './get-import-nodes-matched-group';
import { getSortedImportSpecifiers } from './get-sorted-import-specifiers';
import { getSortedNodesGroup } from './get-sorted-nodes-group';
import { normalizeImportOrderOption } from './normalize-import-order-options';

/**
* This function returns the given nodes, sorted in the order as indicated by
Expand All @@ -19,17 +16,11 @@ export const getSortedNodesByImportOrder: GetSortedNodes = (
originalNodes,
options,
) => {
let { importOrder } = options;
// This normalization is safe even if the option is already correct.
const importOrder = normalizeImportOrderOption(options.importOrder);

const finalNodes: ImportOrLine[] = [];

if (!importOrder.includes(THIRD_PARTY_MODULES_SPECIAL_WORD)) {
importOrder = [THIRD_PARTY_MODULES_SPECIAL_WORD, ...importOrder];
}

// Opinionated decision: builtin modules should always be first
importOrder = [BUILTIN_MODULES, ...importOrder];

const importOrderGroups = importOrder.reduce<ImportGroups>(
(groups, regexp) =>
// Don't create a new group for explicit import separators
Expand Down
33 changes: 33 additions & 0 deletions src/utils/normalize-import-order-options.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import {
BUILTIN_MODULES_REGEX_STR,
BUILTIN_MODULES_SPECIAL_WORD,
THIRD_PARTY_MODULES_SPECIAL_WORD,
} from '../constants';
import { PrettierOptions } from '../types';

export function normalizeImportOrderOption(
importOrder: PrettierOptions['importOrder'],
) {
// THIRD_PARTY_MODULES_SPECIAL_WORD is magic because "everything not matched by other groups goes here"
// So it must always be present.
if (!importOrder.includes(THIRD_PARTY_MODULES_SPECIAL_WORD)) {
importOrder = [THIRD_PARTY_MODULES_SPECIAL_WORD, ...importOrder];
}

// Opinionated Decision: NodeJS Builtin modules should always be separate from third party modules
// Users may want to add their own separators around them or insert other modules above them though
if (
!(
importOrder.includes(BUILTIN_MODULES_SPECIAL_WORD) ||
importOrder.includes(BUILTIN_MODULES_REGEX_STR)
)
) {
importOrder = [BUILTIN_MODULES_SPECIAL_WORD, ...importOrder];
}

importOrder = importOrder.map((g) =>
g === BUILTIN_MODULES_SPECIAL_WORD ? BUILTIN_MODULES_REGEX_STR : g,
);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do this map here, vs just injecting the regex str on line 25?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could inject the regex-str up on line 25, but it wouldn’t allow for us to remove the .map — if we want to allow users to have the built-in modules in a different place.

Here’s my thought process our code sort of looks like this:

// top-of-file-comment(s)
a-z builtin imports
a-z other-third-party
a-z relative imports
runtime.code()

I like gaps between my groups of imports, so we’d have this awkward leading empty string on the array [“”, ”<THIRD_PARTY_MODULES>”,… if there was no word for builtin modules.

And we’d never even be able to express “put a gap above built-ins, but after top-of-file comments” if we didn’t have that special word. — Unfortunately, we still can’t do that (yet) even if importOrder can express it. It would require further logic that I’m still interested in implementing; I think it’s just a (+-)1 on the shifting of the loc you recently added.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, I was neglecting to consider that we need to support both having the special word in the config and injecting it. The third-party special word is handled elsewhere.


return importOrder;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`exercise-custom-builtin-modules-spacing.ts - typescript-verify > exercise-custom-builtin-modules-spacing.ts 1`] = `
// Top-of-file-comment
import path from "path"
import b from 'b';
import foo from './foo';
import thirdParty from 'third-party';
import fs from "node:fs"
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// Top-of-file-comment
import b from "b";
import thirdParty from "third-party";

import fs from "node:fs";
import path from "path";

import foo from "./foo";

`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Top-of-file-comment
import path from "path"
import b from 'b';
import foo from './foo';
import thirdParty from 'third-party';
import fs from "node:fs"
5 changes: 5 additions & 0 deletions tests/ImportOrderBuiltinModulesToCustom/ppsi.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import {run_spec} from '../../test-setup/run_spec';

run_spec(__dirname, ['typescript'], {
importOrder: ['<THIRD_PARTY_MODULES>','','<BUILTIN_MODULES>','','^[./]'],
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
exports[`relative-import-with-builtin-substring.ts - typescript-verify > relative-import-with-builtin-substring.ts 1`] = `
import foo from './constants/foo';
import thirdParty from 'third-party';
import fs from "fs"
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
import fs from "fs";

import thirdParty from "third-party";
import foo from "./constants/foo";

Expand Down
2 changes: 1 addition & 1 deletion tests/ImportOrderBuiltinModulesToTop/ppsi.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {run_spec} from '../../test-setup/run_spec';

run_spec(__dirname, ['typescript'], {
importOrder: ['^[./]'],
importOrder: ['','<THIRD_PARTY_MODULES>','^[./]'],
});
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
import foo from './constants/foo';
import thirdParty from 'third-party';
import fs from "fs"