Skip to content

Conversation

@mjbvz
Copy link
Contributor

@mjbvz mjbvz commented Dec 2, 2025

Remove the need to use tsx here

Copilot AI review requested due to automatic review settings December 2, 2025 00:13
@vs-code-engineering vs-code-engineering bot added this to the November 2025 milestone Dec 2, 2025
@mjbvz mjbvz marked this pull request as draft December 2, 2025 00:16
Matches the version we use in vscode repo
Copilot finished reviewing on behalf of mjbvz December 2, 2025 00:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR attempts to modernize the ESLint plugin architecture by removing the dependency on tsx and directly running TypeScript files using Node.js's experimental TypeScript support. The changes migrate from CommonJS to ES modules, update type assertions to use the as syntax, and leverage modern Node.js features like import.meta.dirname.

Key Changes

  • Migrated custom ESLint rules from CommonJS (export =) to ES modules (export default)
  • Replaced .eslintplugin/index.js with .eslintplugin/index.ts to dynamically load rules using ESM imports
  • Updated all type assertions from angle-bracket syntax (<Type>) to as Type syntax throughout ESLint plugin files

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
eslint.config.mjs Changed to import TypeScript files directly from .eslintplugin/index.ts instead of compiled JavaScript
.eslintplugin/tsconfig.json Updated to ES2024 target with allowImportingTsExtensions and erasableSyntaxOnly for Node.js TypeScript support
.eslintplugin/package.json Added "type": "module" to enable ES module resolution
.eslintplugin/index.ts New file replacing index.js, uses dynamic imports and import.meta.dirname to load rules
.eslintplugin/utils.ts Updated type assertions from <Type> to as Type syntax
.eslintplugin/no-runtime-import.ts Changed export syntax, updated type assertions, migrated from __dirname to import.meta.dirname, added .ts extension to imports
.eslintplugin/*.ts (all rule files) Consistently changed from export = to export default and updated type assertions to as syntax
.eslintplugin/index.js Deleted - replaced with TypeScript implementation

headerEslint.rules.header.meta.schema = false;

import localEslint from './.eslintplugin/index.js';
import * as localEslint from './.eslintplugin/index.ts';
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

Importing .ts files directly in an .mjs file will fail at runtime without additional configuration. Node.js (even version 22.14.0) cannot natively import TypeScript files without either:

  1. The --experimental-strip-types flag, or
  2. A TypeScript loader/transpiler (like tsx which was removed in this PR)

The ESLint CLI will fail to load this config file. You'll need to either:

  • Keep the old approach with a .js file that uses tsx to load TypeScript files
  • Compile the .eslintplugin files to JavaScript and import the compiled .js files
  • Use a different mechanism to load TypeScript-based ESLint rules

Consider adding a build step that compiles .eslintplugin/*.ts to .eslintplugin/dist/*.js and importing from there instead.

Suggested change
import * as localEslint from './.eslintplugin/index.ts';
import * as localEslint from './.eslintplugin/dist/index.js';

Copilot uses AI. Check for mistakes.
@mjbvz mjbvz marked this pull request as ready for review December 5, 2025 16:51
alexr00
alexr00 previously approved these changes Dec 5, 2025
@mjbvz mjbvz added this pull request to the merge queue Dec 5, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 5, 2025
@mjbvz mjbvz added this pull request to the merge queue Dec 5, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 5, 2025
@mjbvz mjbvz added this pull request to the merge queue Dec 5, 2025
Merged via the queue into main with commit 3bfd48e Dec 5, 2025
17 checks passed
@mjbvz mjbvz deleted the dev/mjbvz/asleep-guppy branch December 5, 2025 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants