Skip to content

fix(typescript-estree): ensure mjs/mts files are always be parsed as ESM #10011

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Oct 4, 2024
Merged

fix(typescript-estree): ensure mjs/mts files are always be parsed as ESM #10011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4a1e81b
change mjs/mts files to always be parsed by ESM
goldentrash May 19, 2024
e98727b
revert wrong approach
goldentrash May 22, 2024
0027099
change mjs/mts files to always be parsed as ESM (using setExternalMod…
goldentrash May 22, 2024
7172c05
changed `sourceType` options to be imported from a config file
goldentrash Sep 16, 2024
b34e53f
write test code (`typescript-estree/tests/lib/parse.test.ts`)
goldentrash Sep 16, 2024
d369ec8
change the test to simply not set the `project` property on the `pars…
goldentrash Sep 16, 2024
fa9d25b
doc: update `TypesScript_ESTree`'s `ParseOptions`
goldentrash Sep 30, 2024
138054e
Merge branch 'main' of https://github.com/goldentrash/typescript-esli…
goldentrash Oct 2, 2024
3bdee13
fix eslint error
goldentrash Oct 2, 2024
183d8fc
Merge branch 'main' of https://github.com/goldentrash/typescript-esli…
goldentrash Oct 4, 2024
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
6 changes: 6 additions & 0 deletions docs/packages/TypeScript_ESTree.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ Parses the given string of code with the options provided and returns an ESTree-

```ts
interface ParseOptions {
/**
* Specify the `sourceType`.
* For more details, see https://github.com/typescript-eslint/typescript-eslint/pull/9121
*/
sourceType?: SourceType;

/**
* Prevents the parser from throwing an error if it receives an invalid AST from TypeScript.
* This case only usually occurs when attempting to lint invalid code.
Expand Down
1 change: 1 addition & 0 deletions packages/typescript-estree/src/create-program/createSourceFile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ function createSourceFile(parseSettings: ParseSettings): ts.SourceFile {
{
languageVersion: ts.ScriptTarget.Latest,
jsDocParsingMode: parseSettings.jsDocParsingMode,
setExternalModuleIndicator: parseSettings.setExternalModuleIndicator,
},
/* setParentNodes */ true,
getScriptKind(parseSettings.filePath, parseSettings.jsx),
Expand Down
28 changes: 21 additions & 7 deletions packages/typescript-estree/src/parseSettings/createParseSettings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import path from 'node:path';

import debug from 'debug';
import * as ts from 'typescript';

Expand Down Expand Up @@ -46,6 +48,14 @@ export function createParseSettings(
? tsestreeOptions.tsconfigRootDir
: process.cwd();
const passedLoggerFn = typeof tsestreeOptions.loggerFn === 'function';
const filePath = ensureAbsolutePath(
typeof tsestreeOptions.filePath === 'string' &&
tsestreeOptions.filePath !== '<input>'
Copy link
Contributor

Choose a reason for hiding this comment

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

As I checked <text> still used in ESLint, should we ignore it too? This code exists before this PR, proberly won't cause problems. When I saw this code, I thought maybe they removed <text> placeholder. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought the <text> placeholder was never used because typescript-eslint provides a default value for filepath. The documentation described the code as follows,

const PARSE_DEFAULT_OPTIONS: ParseOptions = {
  comment: false,
  filePath: 'estree.ts', // or 'estree.tsx', if you pass jsx: true
  jsDocParsingMode: 'all',
  jsx: false,
  loc: false,
  loggerFn: undefined,
  range: false,
  tokens: false,
};

declare function parse(
  code: string,
  options: ParseOptions = PARSE_DEFAULT_OPTIONS,
): TSESTree.Program;

But, here's the code I actually found

function parse<T extends TSESTreeOptions = TSESTreeOptions>(
  code: string,
  options?: T,  // there's no default value
): AST<T> {
  const { ast } = parseWithNodeMapsInternal(code, options, false);
  return ast;
}

Maybe it's just something missed over time.

Should this be fixed in the current PR? I feel like the problem we're dealing with is a little different.

? tsestreeOptions.filePath
: getFileName(tsestreeOptions.jsx),
tsconfigRootDir,
);
const extension = path.extname(filePath).toLowerCase() as ts.Extension;
const jsDocParsingMode = ((): ts.JSDocParsingMode => {
switch (tsestreeOptions.jsDocParsingMode) {
case 'all':
Expand Down Expand Up @@ -81,13 +91,17 @@ export function createParseSettings(
tsestreeOptions.extraFileExtensions.every(ext => typeof ext === 'string')
? tsestreeOptions.extraFileExtensions
: [],
filePath: ensureAbsolutePath(
typeof tsestreeOptions.filePath === 'string' &&
tsestreeOptions.filePath !== '<input>'
? tsestreeOptions.filePath
: getFileName(tsestreeOptions.jsx),
tsconfigRootDir,
),
filePath,
setExternalModuleIndicator:
tsestreeOptions.sourceType === 'module' ||
(tsestreeOptions.sourceType === undefined &&
extension === ts.Extension.Mjs) ||
(tsestreeOptions.sourceType === undefined &&
extension === ts.Extension.Mts)
? (file): void => {
file.externalModuleIndicator = true;
}
: undefined,
jsDocParsingMode,
jsx: tsestreeOptions.jsx === true,
loc: tsestreeOptions.loc === true,
Expand Down
8 changes: 8 additions & 0 deletions packages/typescript-estree/src/parseSettings/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ export interface MutableParseSettings {
*/
filePath: string;

/**
* Sets the external module indicator on the source file.
* Used by Typescript to determine if a sourceFile is an external module.
*
* needed to always parsing `mjs`/`mts` files as ESM
*/
setExternalModuleIndicator?: (file: ts.SourceFile) => void;

/**
* JSDoc parsing style to pass through to TypeScript
*/
Expand Down
7 changes: 7 additions & 0 deletions packages/typescript-estree/src/parser-options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type {
DebugLevel,
JSDocParsingMode,
ProjectServiceOptions,
SourceType,
} from '@typescript-eslint/types';
import type * as ts from 'typescript';

Expand All @@ -15,6 +16,12 @@ export type { ProjectServiceOptions } from '@typescript-eslint/types';
//////////////////////////////////////////////////////////

interface ParseOptions {
/**
* Specify the `sourceType`.
* For more details, see https://github.com/typescript-eslint/typescript-eslint/pull/9121
*/
sourceType?: SourceType;

/**
* Prevents the parser from throwing an error if it receives an invalid AST from TypeScript.
* This case only usually occurs when attempting to lint invalid code.
Expand Down
128 changes: 128 additions & 0 deletions packages/typescript-estree/tests/lib/parse.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,134 @@ describe('parseAndGenerateServices', () => {
});
});

describe('ESM parsing', () => {
describe('TLA(Top Level Await)', () => {
const config: TSESTreeOptions = {
projectService: false,
comment: true,
tokens: true,
range: true,
loc: true,
};
const code = 'await(1)';

const testParse = ({
sourceType,
ext,
shouldAllowTLA = false,
}: {
sourceType?: 'module' | 'script';
ext: '.js' | '.ts' | '.mjs' | '.mts';
shouldAllowTLA?: boolean;
}): void => {
const ast = parser.parse(code, {
...config,
sourceType,
filePath: `file${ext}`,
});
const expressionType = (
ast.body[0] as parser.TSESTree.ExpressionStatement
).expression.type;

it(`parse(): should ${
shouldAllowTLA ? 'allow' : 'not allow'
} TLA for ${ext} file with sourceType = ${sourceType}`, () => {
expect(expressionType).toBe(
shouldAllowTLA
? parser.AST_NODE_TYPES.AwaitExpression
: parser.AST_NODE_TYPES.CallExpression,
);
});
};
const testParseAndGenerateServices = ({
sourceType,
ext,
shouldAllowTLA = false,
}: {
sourceType?: 'module' | 'script';
ext: '.js' | '.ts' | '.mjs' | '.mts';
shouldAllowTLA?: boolean;
}): void => {
const result = parser.parseAndGenerateServices(code, {
...config,
sourceType,
filePath: `file${ext}`,
});
const expressionType = (
result.ast.body[0] as parser.TSESTree.ExpressionStatement
).expression.type;

it(`parseAndGenerateServices(): should ${
shouldAllowTLA ? 'allow' : 'not allow'
} TLA for ${ext} file with sourceType = ${sourceType}`, () => {
expect(expressionType).toBe(
shouldAllowTLA
? parser.AST_NODE_TYPES.AwaitExpression
: parser.AST_NODE_TYPES.CallExpression,
);
});
};

testParse({ ext: '.js' });
testParse({ ext: '.ts' });
testParse({ ext: '.mjs', shouldAllowTLA: true });
testParse({ ext: '.mts', shouldAllowTLA: true });

testParse({ sourceType: 'module', ext: '.js', shouldAllowTLA: true });
testParse({ sourceType: 'module', ext: '.ts', shouldAllowTLA: true });
testParse({ sourceType: 'module', ext: '.mjs', shouldAllowTLA: true });
testParse({ sourceType: 'module', ext: '.mts', shouldAllowTLA: true });

testParse({ sourceType: 'script', ext: '.js' });
testParse({ sourceType: 'script', ext: '.ts' });
testParse({ sourceType: 'script', ext: '.mjs' });
testParse({ sourceType: 'script', ext: '.mts' });

testParseAndGenerateServices({ ext: '.js' });
testParseAndGenerateServices({ ext: '.ts' });
testParseAndGenerateServices({ ext: '.mjs', shouldAllowTLA: true });
testParseAndGenerateServices({ ext: '.mts', shouldAllowTLA: true });

testParseAndGenerateServices({
sourceType: 'module',
ext: '.js',
shouldAllowTLA: true,
});
testParseAndGenerateServices({
sourceType: 'module',
ext: '.ts',
shouldAllowTLA: true,
});
testParseAndGenerateServices({
sourceType: 'module',
ext: '.mjs',
shouldAllowTLA: true,
});
testParseAndGenerateServices({
sourceType: 'module',
ext: '.mts',
shouldAllowTLA: true,
});

testParseAndGenerateServices({
sourceType: 'script',
ext: '.js',
});
testParseAndGenerateServices({
sourceType: 'script',
ext: '.ts',
});
testParseAndGenerateServices({
sourceType: 'script',
ext: '.mjs',
});
testParseAndGenerateServices({
sourceType: 'script',
ext: '.mts',
});
});
});

if (process.env.TYPESCRIPT_ESLINT_PROJECT_SERVICE !== 'true') {
describe('invalid file error messages', () => {
const PROJECT_DIR = resolve(FIXTURES_DIR, '../invalidFileErrors');
Expand Down
2 changes: 1 addition & 1 deletion packages/typescript-estree/typings/typescript.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import 'typescript';
// these additions are marked as internal to typescript
declare module 'typescript' {
interface SourceFile {
externalModuleIndicator?: Node;
externalModuleIndicator?: Node | true;
parseDiagnostics: DiagnosticWithLocation[];
}

Expand Down
Loading