Use a bundler or package.json exports to provide path exports (and modernize project)

[eagerestwolf]

Describe the need
So, as addressed (to some extent) in #137 and (for sure) in #146, the current setup for using the plugin is a bit confusing and a little obtuse. There are currently 2 ways to use the plugin (at least for ESLint 8/9), and neither is particularly great.

Obviously, the first way (I'll call the legacy way) is to import the plugin as shown below and call a flat config directly. This way of using the plugin seems to be discouraged though (despite working flawlessly....mostly), This is also the method that most plugin authors have settled on, since it requires basically no change to existing tooling.

• Legacy example:

  // @ts-check
  
  import solid from "eslint-plugin-solid";
  import tseslint from "typescript-eslint";
  
  export default tseslint.config(
    // Other configs
    solid.configs["flat/recommended"]
  );

Conversely, the recommended way to use the plugin doesn't work very well. If you include the .js extension that node requires to even load the module and not cause ESLint to error out, TypeScript will complain what is being imported is not a module and has no types. This is the exact issue that #146 is describing.

• Recommended examples:
  • without .js extension (as shown in docs):

    // @ts-check
    
    // This won't work. This will cause an error from node because the .js extension is required
    import solidRecommended from "eslint-plugin-solid/configs/recommended";
    import tseslint from "typescript-eslint";
    
    export default tseslint.config(
      // Other configs
      solidRecommended
    );

  • with .js extension (required to make the code work, TypeScript now complains):

    // @ts-check
    
    // This will work, but TypeScript isn't happy about it
    import solidRecommeneded from "eslint-plugin-solid/configs/recommended.js";
    import tseslint from "typescript-eslint";
    
    export default tseslint.config(
      // Other configs
      solidRecommended
    );

The reason for this discrepancy is related to how modern node handles modules. If your project is a modules project (i.e. "type": "module"), then .js extensions are required. It's complicated, but that's how things work now. Here's a page on the node docs if you want to read more. So why then is TypeScript complaining that what you just imported with that .js extension is not a module...because it's not. If you open the source .js file and look at it, it's not a module. It's CommonJS. Welcome to the wacky world of mixing ESModules and CommonJS.

Additionally, there are some other...issues...with the codebase. First, the .nvmrc calls for Node version 14.15.4, which has long been deprecated, and isn't supported by ESLint, Solid, or PNPM anymore (I suspect nobody tested the repo with nvm --use-on-cd). Officially, the lowest version of node supported by those 3 is 18.18.x. Next, there is still a peer dependency warning when installing eslint-plugin-solid with ESLint 9 because the version of @typescript-eslint/utils used (no harm there, the new version has only been out for 3 days). I'm not trying to be overly critical here, I'm just calling out all the issues I see, and DevOps is kind of my thing.

Suggested Solution
From what I can tell, there are a few paths forward. I'm going to discuss what I think should happen here, and we can go from there (that's why I opened an issue and not a PR). I am willing to do all this work myself. I will walk through each change and explain why I think that's the best path forward. (NOTE: the changes I am going to suggest would normally result in a major version bump, but since this project is not 1.0 yet, it'll probably be a minor bump)

• Update the .nvmrc to call for version18.20.4
  • This is currently the oldest supported LTS version. This does mean that the bar will need to be moved each time the LTS changes, but that isn't super frequent
  • This also keeps the plugin in sync with what PNPM, ESLint, and Solid officially support
• Drop the standalone version
  • It's literally not even mentioned in the docs (I legitimately didn't even know there was a standalone version until browsing the source)
• Update @typescript-eslint/utils to version 8
  • 🎉 no more peer dep warnings yay
• Add a packageManager to package.json
  • That's just the thing you do these days. Corepack (available in Node 18+) will automatically install the correct package manager
• Drop the separate flat configs (don't worry, TypeScript has your back on this one)
  • This will require a non-trivial rewrite of the actual plugin (just index.ts), but the results will be worth it in my opinion
• Bring in tsup
  • Wait, shouldn't dropping the weird configs directory negate this? Kind of. ESLint now officially supports CommonJS and ESModule configuration files, so really you should (although you aren't required to) ship CommonJS and ESModule code and tsup can automate this.
  • This ensures the types are properly registered no matter how the plugin is imported (CommonJS or ESModules)
  • This also properly registers the plugin in both module systems (at the cost of modifying your package.json)
• Add prettier-plugin-pkg (just a personal preference, but it keeps your package.json sorted in the "official" order specified on the NPM docs)

Possible Alternatives
Technically, the standalone version could still exist, but shipping a patched version of ESLint seems like a bad idea. Additionally, I doubt it gets much use. In the same vain, it's technically possible to keep both forms of the flat configs, but it's probably best to pick one or the other and stick to it and most other plugins have settled on extending the existing plugin (I provide some examples in the additional context below). There is 1 notable exception I can think of...Prettier, but Prettier has kind of always done their own thing.

Additional context
Plugins that are adding flat configs to the plugin itself (while these are cherry picked, they are cherry picked in that these are some of the more popular ESLint plugins):

• eslint-plugin-unicorn
• eslint-plugin-jsdoc
• eslint-plugin-tailwindcss
• eslint-plugin-perfectionist
• @stylistic/eslint-plugin

• I would be willing to contribute a PR to implement this feature

[pauliesnug]

I think a mixture of those solutions is the best way to move forward. I doubt anyone is using the standalone version other than internally, so it can just be copied over into those internal plugins. Bumping typescript-eslint to v8 and the Node version makes sense, and there is already an easy PR for one of them. Tsup is a great bundler, which many plugins use.

[pauliesnug]

This would make developing the plugin easier and also remove any peer dependency warnings/TypeScript complaints (considering eslint.config.ts was added in the last ESLint update)

[eagerestwolf]

For sure. Using tsup would also remove the need for having a separate tsconfig.build.json since (for the most part) tsup ignores the tsconfig anyway. Having both CJS and ES exports would also help with the eslint.config.ts since TypeScript generally plays better with typed ES modules than typed CJS. I have started tinkering around a little bit, and the TSESLint references can be removed as that problem is fixed. Unfortunately, there is now a similar problem with scope-manager in the CompatContext. Sadly, I am not familiar enough with the internal workings of ESLint to truly understand what the CompatContext is doing.

I do have an idea that I think will strike a great balance here. What if we kept the concerns consolidated since the standalone linter is still relevant to the ESLint plugin, and just convert this into a monorepo with both eslint-plugin-solid and eslint-solid-standalone as packages in a packages directory. That way both projects can be updated in tandem since they both, to some extent, depend on each other.

One more suggestion that feels appropriate so as to avoid breaking things any more than is absolutely necessary. I have thought on things and perhaps keeping both ways of using the configs in place for now is prudent, just document both of them, and then later on down the road (once Solid is more prevalent) decide which system to keep at that time.

[joshwilsonvu]

Thanks for all your thought and effort in writing this up. Some context in why the package is structured this way:

Standalone

eslint-solid-standalone isn't meant to be used by ordinary projects; its main (and probably only) purpose is to power linting on https://playground.solidjs.com.

ESLint (at least v8) claims that importing only its Linter class is all you need to do to be able to use ESLint in a non-Node environment, but if you look at standalone/rollup.config.mjs, you'll see how much complexity it takes to strip out the parts of ESLint, typescript-eslint, and their deps that don't work outside Node. I gave ESLint v9 a shot but lots of different things broke and I felt it wasn't worth the effort.

Furthermore, all of those custom build-time transformations can break any time eslint-plugin-solid or those packages are updated. So it's most practical to keep it in this repo, rather than in https://github.com/solidjs/solid-playground.

Why not a monorepo then? It's chiefly because I want the README at the root to be the plugin's README, not just one for the workspace root, for a good experience visiting the repo. But if we can reasonably deal with that, converting to a monorepo would be great.

ESM / "exports" / CJS

Until v9, ESLint has been a stick in the mud about ESM support. And upgrading to v9 is a challenge. So for as long as it's relatively feasible, I see no reason not to support version 8 and below (I run tests down to v6).

I've written about why I want to support import config from 'eslint-plugin-solid/configs/recommended, not just plugins.configs['flat/...'], here.

It's a little unclear to me whether switching to use package.json "exports" instead of exposing "main" and implicitly exposing nested paths like /configs/recommended.js can worth with ESLint v8's CJS focus. I certainly can't expose ESM only and expect older setups to work. But I guess even with CJS, currently supported Node versions support "exports", right?

It sounds like tsup is the way to go for exposing both CJS and ESM. With multiple entry points, it could support /configs/*. Definitely open to using it for the plugin, maybe not standalone since that's more fragile.

Other

• .nvmrc - had no idea this was important 😅 might just remove it, since any current Node version should work fine
• @typescript-eslint/utils v8 - done in fix: allow typescript-eslint@8 as dependency #149
• "packageManager" - done in fix: allow typescript-eslint@8 as dependency #149

Appreciate any thoughts on this. Thanks again!

[joshwilsonvu]

Converted to a monorepo in #150 🎉