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

refactor: backport a few upstream changes #171

Merged
merged 12 commits into from
Oct 30, 2024
5 changes: 5 additions & 0 deletions .changeset/fast-carpets-teach.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-import-x": patch
---

Perf: avoid regexp during parser choosing
5 changes: 5 additions & 0 deletions .changeset/hot-jokes-grab.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-import-x": patch
---

Add extra guard for rule `no-named-as-default`. A few guards are borrowed from https://github.com/import-js/eslint-plugin-import/pull/3032, but we don't sync the rest of changes from upstream since we have already implemented a way more performant check.
5 changes: 5 additions & 0 deletions .changeset/old-years-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-import-x": patch
---

More test cases for `no-named-export` and `no-defualt-export` rule specifically with non-module `sourceType`
5 changes: 5 additions & 0 deletions .changeset/orange-dryers-mix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-import-x": patch
---

Fix `export` when there is only one `TSDeclareFunction` (https://github.com/import-js/eslint-plugin-import/pull/3065)
5 changes: 5 additions & 0 deletions .changeset/shaggy-mayflies-applaud.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-import-x": patch
---

Prevent `ExportMap`'s cache is being tainted by incompatible parser (e.g. old `babel-eslint`). The cache is now skipped w/ incompatible parsers, which might introduce performance impacts only for those who are using incompatible parsers. (https://github.com/import-js/eslint-plugin-import/pull/3062)
5 changes: 5 additions & 0 deletions .changeset/spicy-mangos-pull.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-import-x": patch
---

Docs: fix a few typos here and there
5 changes: 5 additions & 0 deletions .changeset/tough-elephants-divide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-import-x": patch
---

Properly fix espree parser w/ ESLint Flat Config
3 changes: 1 addition & 2 deletions docs/rules/no-relative-packages.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@

Use this rule to prevent importing packages through relative paths.

It's useful in Yarn/Lerna workspaces, were it's possible to import a sibling
package using `../package` relative path, while direct `package` is the correct one.
It's useful in a monorepo setup, where it's possible to import a sibling package using `../package` relative path, while direct `package` is the correct one.

## Examples

Expand Down
2 changes: 1 addition & 1 deletion docs/rules/no-restricted-paths.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
Some projects contain files which are not always meant to be executed in the same environment.
For example consider a web application that contains specific code for the server and some specific code for the browser/client. In this case you don’t want to import server-only files in your client code.

In order to prevent such scenarios this rule allows you to define restricted zones where you can forbid files from imported if they match a specific path.
In order to prevent such scenarios this rule allows you to define restricted zones where you can forbid files from being imported if they match a specific path.

## Rule Details

Expand Down
47 changes: 19 additions & 28 deletions src/rules/export.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { AST_NODE_TYPES } from '@typescript-eslint/utils'
import type { TSESTree } from '@typescript-eslint/utils'

import {
Expand Down Expand Up @@ -30,37 +31,24 @@ const rootProgram = 'root'
const tsTypePrefix = 'type:'

/**
* Detect function overloads like:
* Remove function overloads like:
*
* ```ts
* export function foo(a: number);
* export function foo(a: string);
* export function foo(a: number|string) { return a; }
* ```
*/
function isTypescriptFunctionOverloads(nodes: Set<TSESTree.Node>) {
const nodesArr = [...nodes]

const idents = nodesArr.flatMap(node =>
'declaration' in node && node.declaration?.type === 'TSDeclareFunction'
? node.declaration.id!.name
: [],
)

if (new Set(idents).size !== idents.length) {
return true
}

const types = new Set(nodesArr.map(node => `${node.parent!.type}` as const))
if (!types.has('TSDeclareFunction')) {
return false
}
if (types.size === 1) {
return true
}
if (types.size === 2 && types.has('FunctionDeclaration')) {
return true
function removeTypescriptFunctionOverloads(nodes: Set<TSESTree.Node>) {
for (const node of nodes) {
const declType =
node.type === AST_NODE_TYPES.ExportDefaultDeclaration
? node.declaration.type
: node.parent?.type
if (declType === AST_NODE_TYPES.TSDeclareFunction) {
nodes.delete(node)
}
}
return false
}

/**
Expand Down Expand Up @@ -277,14 +265,17 @@ export = createRule<[], MessageId>({
'Program:exit'() {
for (const [, named] of namespace) {
for (const [name, nodes] of named) {
if (nodes.size === 0) {
continue
}

removeTypescriptFunctionOverloads(nodes)

if (nodes.size <= 1) {
continue
}

if (
isTypescriptFunctionOverloads(nodes) ||
isTypescriptNamespaceMerging(nodes)
) {
if (isTypescriptNamespaceMerging(nodes)) {
continue
}

Expand Down
11 changes: 11 additions & 0 deletions src/rules/no-named-as-default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,17 @@ export = createRule<[], MessageId>({
return
}

if (!exportMapOfImported.hasDefault) {
// The rule is triggered for default imports/exports, so if the imported module has no default
// this means we're dealing with incorrect source code anyway
return
}

if (!exportMapOfImported.has(nameValue)) {
// The name used locally for the default import was not even used in the imported module.
return
}

if (
exportMapOfImported.exports.has('default') &&
exportMapOfImported.exports.has(nameValue)
Expand Down
6 changes: 5 additions & 1 deletion src/utils/export-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,11 @@ export class ExportMap {

exportMap.mtime = stats().mtime.valueOf()

exportCache.set(cacheKey, exportMap)
// If the visitor keys were not populated, then we shouldn't save anything to the cache,
// since the parse results may not be reliable.
if (exportMap.visitorKeys) {
exportCache.set(cacheKey, exportMap)
}

return exportMap
}
Expand Down
12 changes: 9 additions & 3 deletions src/utils/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,22 @@ function withoutProjectParserOptions(
const log = debug('eslint-plugin-import-x:parse')

function keysFromParser(
parserPath: string | TSESLint.Parser.ParserModule,
_parserPath: string | TSESLint.Parser.ParserModule,
parserInstance: TSESLint.Parser.ParserModule,
parsedResult?: TSESLint.Parser.ParseResult,
) {
// Exposed by @typescript-eslint/parser and @babel/eslint-parser
if (parsedResult && parsedResult.visitorKeys) {
return parsedResult.visitorKeys
}
if (typeof parserPath === 'string' && /.*espree.*/.test(parserPath)) {
// @ts-expect-error - no type yet

// The espree parser doesn't have the `parseForESLint` function, so we don't ended up with a
// `parsedResult` here, but it does expose the visitor keys on the parser instance that we can use.
if (
parserInstance &&
'VisitorKeys' in parserInstance &&
parserInstance.VisitorKeys
) {
return parserInstance.VisitorKeys as TSESLint.SourceCode.VisitorKeys
}
return null
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/order-redirect-scoped/module/package.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "order-redirect-module",
"name": "order-redirect-module-scoped",
"private": true,
"main": "../other-module/file.js"
}
21 changes: 19 additions & 2 deletions test/rules/export.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ ruleTester.run('export', rule, {
}
`,
},
{
code: `
export default function foo(param: string): boolean;
export default function foo(param: string, param1?: number): boolean {
return param && param1;
}
`,
},
],

invalid: [
Expand Down Expand Up @@ -160,6 +168,15 @@ ruleTester.run('export', rule, {
},
},
}),

test({
code: `
export default function a(): void;
export default function a() {}
export { x as default };
`,
errors: ['Multiple default exports.', 'Multiple default exports.'],
}),
],
})

Expand Down Expand Up @@ -510,7 +527,7 @@ describe('TypeScript', () => {
}),
test({
code: `
export function Foo();
export function Foo() { };
export class Foo { }
export namespace Foo { }
`,
Expand All @@ -529,7 +546,7 @@ describe('TypeScript', () => {
test({
code: `
export const Foo = 'bar';
export function Foo();
export function Foo() { };
export namespace Foo { }
`,
errors: [
Expand Down
2 changes: 1 addition & 1 deletion test/rules/no-cycle.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ const error = (message: string) => ({ message })

const test = <T extends ValidTestCase>(def: T) =>
_test({
...def,
filename: testFilePath('./cycles/depth-zero.js'),
...def,
})

ruleTester.run('no-cycle', rule, {
Expand Down
11 changes: 11 additions & 0 deletions test/rules/no-default-export.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@ const ruleTester = new TSESLintRuleTester()

ruleTester.run('no-default-export', rule, {
valid: [
test({
code: 'module.exports = function foo() {}',
languageOptions: {
parserOptions: {
sourceType: 'script',
},
},
}),
test({
code: 'module.exports = function foo() {}',
}),
test({
code: `
export const foo = 'foo';
Expand Down
11 changes: 11 additions & 0 deletions test/rules/no-named-export.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@ const ruleTester = new TSESLintRuleTester()

ruleTester.run('no-named-export', rule, {
valid: [
test({
code: 'module.export.foo = function () {}',
}),
test({
code: 'module.exports = function foo() {}',
languageOptions: {
parserOptions: {
sourceType: 'script',
},
},
}),
test({
code: 'export default function bar() {};',
}),
Expand Down
11 changes: 11 additions & 0 deletions test/utils/export-map.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@ describe('ExportMap', () => {
)
})

it('does not return a cached copy if the parse does not yield a visitor keys', function () {
const mockContext = {
...fakeContext,
parserPath: 'not-real',
}
expect(ExportMap.get('./named-exports', mockContext)).toBeDefined()
expect(ExportMap.get('./named-exports', mockContext)).not.toBe(
ExportMap.get('./named-exports', mockContext),
)
})

it('does not return a cached copy after modification', done => {
const firstAccess = ExportMap.get('./mutator', fakeContext)
expect(firstAccess).toBeDefined()
Expand Down
Loading