typescript-eslint icon indicating copy to clipboard operation
typescript-eslint copied to clipboard

Feature request: Ship ESM version of `@typescript-eslint/typescript-estree`

Open fisker opened this issue 1 year ago • 4 comments

Before You File a Proposal Please Confirm You Have Done The Following...

Relevant Package

typescript-estree

My proposal is suitable for this project

  • [X] I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Description

Prettier has been try to reduce the size of our typescript plugin, it's not a easy job,

https://github.com/prettier/prettier/blob/a8a0f2f36adb4bf4c89a93dc0ec0a0b4a4f15e06/scripts/build/config.js#L114-L299 https://github.com/prettier/prettier/blob/a8a0f2f36adb4bf4c89a93dc0ec0a0b4a4f15e06/scripts/build/build-javascript-module.js#L57-L70

It will be nice if we can ship @typescript-eslint/typescript-estree with native ESM support.

Additional Info

With the new ESLint flat config, user can also use import to use the ESLint plugin now, please consider accept 🙏

fisker avatar Jun 13 '24 02:06 fisker

With the new ESLint flat config, user can also use import to use the ESLint plugin now

If they use one of

  • flat config with eslint.config.mjs
  • flat config with eslint.config.js and "type": "module"

Then sure - they can use import!

However if they use one of

  • flat config with eslint.config.cjs
  • flat config with eslint.config.js with no "type" or "type": "commonjs"
  • legacy config in any form [^1] [^2] [^3]
  • legacy config with eslint-plugin-import with a rule that requires parsing dependencies [^4] (similar for eslint-plugin-import-x [^5])
  • vue-eslint-parser configured with a string for parserOptions.parser [^6]

Then the user cannot use import and must use require!

Legacy configs usecases are the big one - if we wish to support legacy configs then we must continue to export our plugin and parser as CJS. Additionally our utils package is used by many other plugins - so if we switch that to ESM then those plugins too be forced to switch to ESM as well - which is a big thing to force on the ecosystem and will rustle a lot of jimmies (reference: sindresorhus switching their packages to ESM).

Switching to ESM for typescript-estree would force us to switch our entire repo to ESM because it's a package that underpins pretty well every other package in our project. ESLint doesn't support async parsing nor does it support async rules - so everything needs to be sync.

[^1]: plugin loading: https://github.com/eslint/eslintrc/blob/fc9837d3c4eb6c8c97bd5594fb72ea95b6e32ab8/lib/config-array-factory.js#L1087 [^2]: parser loading: https://github.com/eslint/eslintrc/blob/fc9837d3c4eb6c8c97bd5594fb72ea95b6e32ab8/lib/config-array-factory.js#L984 [^3]: plugin/parser path resolution: https://github.com/eslint/eslintrc/blob/fc9837d3c4eb6c8c97bd5594fb72ea95b6e32ab8/lib/shared/relative-module-resolver.js#L23 [^4]: https://github.com/import-js/eslint-plugin-import/blob/6554bd5c30976290024cecc44ef1e96746cf3cf7/utils/module-require.js [^5]: https://github.com/un-ts/eslint-plugin-import-x/blob/3abe5e49683e0f973232bb631814b935e1ca7091/src/utils/module-require.ts [^6]: https://github.com/vuejs/vue-eslint-parser/blob/b0e0ccc6d302bb40c5cb496528536bd355ee5151/src/script/index.ts#L572

bradzacher avatar Jun 13 '24 03:06 bradzacher

What would ESM net you?

bradzacher avatar Jun 13 '24 03:06 bradzacher

Switching to ESM for typescript-estree would force us to switch our entire repo to ESM

Sorry for not been clear, I didn't mean to stop supporting it been require()d, I mean can we ship it in dual mode (both esm and cjs)?

What would ESM net you?

Esbuid(and other bundlers) is not able to remove unused code from cjs version, that's why Prettier do a lots of weird stuff during build process, I believe esm version can help us remove these hacks.

fisker avatar Jun 13 '24 04:06 fisker

I mean can we ship it in dual mode (both esm and cjs)?

We don't currently have the infra to do this - it would be a non-trivial chunk of work to do this given we don't bundle our code. We would need to ship two copies of our dist folder which doubles the size of our npm package to a megabyte or so.

I'm not sure if it's a good idea going down that route just to decrease your bundle size would be the best approach right now - we don't have any other usecases for ESM and no other packages would need be deployed as ESM or dual mode.

bradzacher avatar Jun 13 '24 08:06 bradzacher

Yeah, sorry, we're not going to be able to take this on anytime soon 😞. We just don't have the maintenance budget to dual-emit CJS+ESM anytime soon - we barely have enough budget to get ourselves onto Nx proper this year. Plus we're still cleaning up messes from the rough ESLint 9 migration and fleshing out the project service work.

Even if we did have the budget to dual-emit, we wouldn't want to double the install size of all our packages for all consumers.

Long-term (think 2025+), it'd be nice if we could altogether switch to ESM. But we'd need to consider the rest of the ESLint ecosystem in that:

  • Once ESLint v10/+ drops legacy config support, will the rest of the ecosystem accelerate switching to ESM?
    • Is our lack of ESM emit actually a blocker/hindrance there?
  • Can we see what the experience is from smaller, less depended-upon packages would be first, ideally?

So, closing for now, but something to keep in mind for post-ESLint-10. ❤️

JoshuaKGoldberg avatar Aug 19 '24 12:08 JoshuaKGoldberg