solid-playground icon indicating copy to clipboard operation
solid-playground copied to clipboard

Add ESLint support

Open joshwilsonvu opened this issue 3 years ago • 7 comments

Though we already have TypeScript support in the playground, I think there are use cases where ESLint really shines. I'm the author of eslint-plugin-solid and I think it would help people learn to write Solid code with fewer surprises.

I would plan to submit a PR for this. I just wanted to check if that's something you would be likely to accept.

Details:

  • We would have to hardcode a list of lint rules to use, it's not easily user-configurable in a browser context since some special measures have to be taken to run ESLint in a browser.
  • Some users may not want to run ESLint, so I'd add a toggle to turn it off and on. It should probably be default on to be of any use, but I'd be happy to ship it as default off initially.
  • It's not quite v1-ready, so I understand wanting to hold off, but I think the toggle should help with any issues.

Is there anything else you want to hear from me about this? Thanks!

joshwilsonvu avatar May 28 '22 13:05 joshwilsonvu

@modderme123 @Jutanium @amoutonbrady do either of you have opinions on this? I think it's a great idea. I would however like to see the ability to disable it.

@joshwilsonvu thank you so much for contributing this amazing plugin. We've been discussing it internally. It's a critical piece of ecosystem!

davedbase avatar May 28 '22 16:05 davedbase

Would appreciate this as a toggleable thing!

Jutanium avatar May 28 '22 16:05 Jutanium

I would love eslint in the playground!

If it would be possible to have eslint run in a worker or be bundled separately, that would be particularly awesome!

milomg avatar May 28 '22 21:05 milomg

I've been working on this from time to time, but since the ESLint ecosystem is currently so tied to CommonJS, it's been a difficult time getting it to work with Vite, even if it's possible with Webpack or something else. I've come to believe that the only way it might be possible to add ESLint to the playground would be to bundle everything including eslint-plugin-solid with a CJS-friendly bundler and use it here as a dependency (like @joshwilsonvu/eslint-solid-worker).

I'm pretty confident that you don't want to add a second build to this repo. I'm not sure if this feature is worth it to you to depend on a new package you don't control? Please let me know if you would still consider accepting a PR as described. Thank you!

joshwilsonvu avatar Jul 11 '22 02:07 joshwilsonvu

Hmm, you're right that I typically try to use the build tools to their fullest extend before adding a new step. I'm open to a package, but worried that it may get out of date.

What do you think about a vite plugin that calls esbuild to do your cjs->esm bundle step?

Also see the typescript-eslint playground

milomg avatar Jul 11 '22 08:07 milomg

I'm open to a package, but worried that it may get out of date.

Do you mean out of sync with eslint-plugin-solid, or the greater ecosystem? For the former, I'll automate the process of publishing the new package whenever I update eslint-plugin-solid.

I looked at typescript-eslint's playground and they actually use the same approach—they (privately) build @typescript-eslint/website-eslint with Rollup in its own workspace and depend on it in the website.

I don't think ESBuild will work here, since it doesn't statically resolve require.resolve() calls. Even with a Vite plugin that uses Rollup, we'd just be emulating Vite's prebundling step, so it feels redundant, but I can try that if you feel strongly about it. For now I'll work on setting up a new package.

joshwilsonvu avatar Jul 23 '22 00:07 joshwilsonvu

Ok, a separate package sounds good then!

milomg avatar Jul 23 '22 00:07 milomg