Thanks for the PR, @goldentrash!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	385bedb
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/66eac469ca55710009ed7600
😎 Deploy Preview	https://deploy-preview-9121--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 99 (🟢 up 4 from production) Accessibility: 100 (no change from production) Best Practices: 92 (no change from production) SEO: 90 (no change from production) PWA: 80 (no change from production) View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 0027099. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

Sent with 💌 from NxCloud.

Hi @bradzacher , I've spent the last few days thinking of other ways to achieve our goal without modifying the code.
From the typescript playground, I knew that ts can properly recognize mjs/mts files as ESM, so I tried to analyze the Typescript code a bit more.

I found the code below and it seems to be acceptable.

/** @internal */
export function getSetExternalModuleIndicator(options: CompilerOptions): (file: SourceFile) => void {
    // TODO: Should this callback be cached?
    switch (getEmitModuleDetectionKind(options)) {
        case ModuleDetectionKind.Force:
            // All non-declaration files are modules, declaration files still do the usual isFileProbablyExternalModule
            return (file: SourceFile) => {
                file.externalModuleIndicator = isFileProbablyExternalModule(file) || !file.isDeclarationFile || undefined;
            };
        case ModuleDetectionKind.Legacy:
            // Files are modules if they have imports, exports, or import.meta
            return (file: SourceFile) => {
                file.externalModuleIndicator = isFileProbablyExternalModule(file);
            };
        case ModuleDetectionKind.Auto:
            // If module is nodenext or node16, all esm format files are modules
            // If jsx is react-jsx or react-jsxdev then jsx tags force module-ness
            // otherwise, the presence of import or export statments (or import.meta) implies module-ness
            const checks: ((file: SourceFile, options: CompilerOptions) => Node | true | undefined)[] = [isFileProbablyExternalModule];
            if (options.jsx === JsxEmit.ReactJSX || options.jsx === JsxEmit.ReactJSXDev) {
                checks.push(isFileModuleFromUsingJSXTag);
            }
            checks.push(isFileForcedToBeModuleByFormat);
            const combined = or(...checks);
            const callback = (file: SourceFile) => void (file.externalModuleIndicator = combined(file, options));
            return callback;
    }
}

Would this be acceptable?

Hi! I'm always ready to update the PR if I need to change my solution to fix this bug!
I'll keep an email alarm on for reviews :)

Does the detection need to agree with what the TypeScript compiler would do given the file extension and tsconfig options? If so, you do need to implement something like that snippet you found in TS that considers the computed value of --moduleDetection.

Does the detection need to agree with what the TypeScript compiler would do given the file extension and tsconfig options? If so, you do need to implement something like that snippet you found in TS that considers the computed value of --moduleDetection.

I think it needs to reflect what TSC sees as you said, and I'll try to modify the PR to take into account things like what you mentioned(-- moduleDetection). I'll have to analyze the TS repo and the typescript-eslint repo a bit more.

Thanks for the review! :)

Unfortunately, I think you’re essentially going to have to choose between porting some TS implementation details (i.e. what options imply what other options) and accessing internal functions 😕

Thanks for the advice @andrewbranch!

I think we need to decide something else first to solve this problem: "Which should be dominant in the settings for the module, eslint.config.js or tsconfig.json?"

My suggestion would be to give eslint.config.js a higher precedence. This is because in this case we can utilize ParserOptions.sourceType.

1. In packages/parser/src/parser.ts/parseForESLint, add sourceType to parserOptions

   function parseForESLint(
     code: ts.SourceFile | string,
     options?: ParserOptions | null,
   ): ParseForESLintResult {
     // https://eslint.org/docs/user-guide/configuring#specifying-parser-options
     // if sourceType is not provided by default eslint expect that it will be set to "script"
     if (options.sourceType !== 'module' && options.sourceType !== 'script') {
       options.sourceType = 'script';
     }
   
     const parserOptions: TSESTreeOptions = {};
     parserOptions.sourceType = options.sourceType;  // like this!
   }

2. In packages/typescript-estree/src/parseSettings/createParseSettings.ts/createParseSettings, set setExternalModuleIndicator based on options.sourceType

   export function createParseSettings(
     code: ts.SourceFile | string,
     options: Partial<TSESTreeOptions> = {},
   ): MutableParseSettings {
       setExternalModuleIndicator:
         options.sourceType === 'module' ||
         extension === ts.Extension.Mjs ||
         extension === ts.Extension.Mts
           ? (file): void => {
               file.externalModuleIndicator = true;
             }
           : undefined,
       ...
   }

Incidentally, if we do this, how about adding a rule to "warn if the module settings in eslint.config.js and tsconfig.json are different"?

👋 Ping @goldentrash - is this still something you have time & energy for?

👋 Ping @goldentrash - is this still something you have time & energy for?

I'm so sorry for the late response. I'm interested in solving this problem! However, I've been so busy with personal matters over the past three weeks that I haven't been able to give it much attention. I'm really sorry for not responding to this, but thankfully, I'm finally able to wrap up all of my pressing matters, so I can continue my challenge to solve this problem this weekend. I'm still interested in this issue, and I'm willing to continue the challenge to resolve it. Thank you.

No worries, hope things are ok!

Sorry for the late response, @bradzacher. As discussed above, I modified the code to determine the sourceType based on the settings in eslint.config.js rather than tsconfig.json, and wrote the associated tests. There are a few things I need to ask, so I'll leave a comment.

1. There are three tests that were requested. I first wrote a test under typescript-estree/tests/lib/parse.test.ts that checks for TLA-allowed parsing based on sourceType and extension. (I thought this was the type-aware thing you were referring to.) However, I didn't quite understand your intent when you mentioned Project Service, so I tested parseAndGenerateServices using the project attribute in /tsconfig.json. Also, I checked the file typescript-estree/tests/lib/parse.project-true.test.ts and it seems to validate parent & sibling project detection regardless of ESM parsing, so I didn't write any additional tests. If you can be a bit more specific, I'll write those tests as well.
2. I've added a new property (sourceType) to typescript-estree/src/parser-options.ts. Based on the comments, it looks like we need to modify the documentation accordingly. Do I need to modify the relevant pages for this as well, and if so, which files?
3. If you look at parser/src/parser.ts, you'll see code like this

   const parserOptions: TSESTreeOptions = {};
   Object.assign(parserOptions, options, {
     sourceType: options.sourceType,     // Here's some code that didn't exist before.
   });
   
   ...
   
   const { ast, services } = parseAndGenerateServices(code, parserOptions);
   ast.sourceType = options.sourceType;  // Is this the right place for this code?

   I was thinking about moving the last line, ast.sourceType = options.sourceType;, inside parseAndGenerateServices, but I was worried about side effects I wasn't aware of, so I left it unmodified for now and kept the existing structure. Should I move that code inside parseAndGenerateServices() and parse()? (The two functions don't reference each other, so if I move it, I'd have to put it inside both of them.) If not, should I modify astConverter in typescript-estree/src/ast-converter.ts to fulfill that role together? Or is the status quo still fine?
4. Finally, my branch now has a conflict with main. Should I resolve that conflict myself? If so, which method should I use? Rebase for linear history or merge?

However, I didn't quite understand your intent when you mentioned Project Service, so I tested parseAndGenerateServices using the project attribute in /tsconfig.json.

I modified the parseAndGenerateServices test to not use the project attribute. It made sense to test the same thing as the parse test, and when I checked the testParse helper in the "isolated parsing" part of the same test file, the attribute didn't seem important to verify the behavior.

First and foremost, the test you mentioned was written with the intent of Project Service in mind, so I thought it was more important to accurately reflect the intent based on your feedback.

I'm sorry. I accidentally closed this PR and when I tried to reopen it, it was impossible, so I opened a new one. Please see the following PR (#10011)