refactor(eslint)!: upgrade eslint to newest version

[colingm]

Rollup Plugin Name: plugin-eslint

This PR contains:

• [ ] bugfix
• [ ] feature
• [x] refactor
• [x] documentation
• [ ] other

Are tests included?

• [x] yes (bugfixes and features will not be merged without tests)
• [ ] no

Breaking Changes?

• [x] yes (breaking changes will not be merged unless absolutely necessary)
• [ ] no

List any relevant issue numbers:

• https://github.com/rollup/plugins/issues/1233
• https://github.com/rollup/plugins/issues/1231
• https://github.com/rollup/plugins/issues/1128
• https://github.com/rollup/plugins/issues/1010
• https://github.com/rollup/plugins/issues/796

Description

Prior to these changes, the eslint plugin was using an older version of eslint that relied on a class called CLIEngine. In newer versions this has now been deprecated and so upgrading versions required also updating to using ESLint internally instead. Ultimately this doesn't change the usage of the plugin but some of the option names have changed for ESLint (notably configFile needs to change to overrideConfigFile).

The main driving factor for this change is to prevent conflicts that happen when using @rollup/plugin-typescript and this plugin together. Without this update, eslint would run on the transpiled code and so the error messages weren't as helpful. This update will allow the two plugins to work well together and provide helpful information about errors and warnings.

BREAKING CHANGES:

• options structure has changed due to moving from CLIEngine to ESlint. Review the new supported options below: https://eslint.org/docs/latest/developer-guide/nodejs-api#-new-eslintoptions

This work is also based on this abandoned pr: https://github.com/rollup/plugins/pull/663

[colingm]

@shellscape I will update the readme and the engine. Quick question about working with pnpm, I noticed there were a ton of changes to the pnpm-lock.yaml and I didn't want to commit some of them (things like the image below). Should I just not worry about those and commit all changes to that file?

[shellscape]

Good eye. Those are because we're still on the lockfile format from pnpm v6.34.0. I have it on my list to update that, but haven't gotten around to it yet. You'll want to use pnpm v6.34.0 to update the lockfile (run pnpm install from the repo root). I'd recommend checking the lockfile out from master first.

[shellscape]

Looks like you might need to run pnpm lint

[colingm]

any ideas?

[shellscape]

You're going down the rabbit hole now. There may be a version conflict between the version of eslint installed for the repo, and the version installed for the rollup eslint plugin. We're hoisting eslint in the monorepo to make it work correctly across all the projects. The version installed for the repo is a direct dep of https://github.com/rollup/eslint-config-rollup/blob/master/package.json#L28.

I'll update that later today and we'll try running CI here again

[colingm]

yeah it looks like it is because @typescript-eslint: 4.90 in plugins only works with eslint v7, with this update I would need to update @typescript-eslint to v5

[shellscape]

Yeah I'll get that updated. It's about time anyhow

[colingm]

I'm sure you will find this out soon enough but as a heads up it looks like just a straight upgrade of @typescript-eslint results in quite a few things popping up for me

[shellscape]

Turns out my editor has a plugin that doesn't support ESLint v8 so I'm working on fixing that now. Then I'll get our eslint config updated, update master, and then you can rebase from upstream. I'll keep you posted.

[shellscape]

Updating ESLint nuked my editor (Atom) ability to format on the fly, so now I'm trying to fix an abandoned prettier-atom plugin lol. Rabbit hole.

[shellscape]

Tried to fix the config issues in your fork, but it's going to require a master branch fix. Will keep you posted.

[colingm]

@shellscape seems like I have cursed you 😅 if you want I can work on updating the @typescript-eslint in my fork as well as long as you have some guidance for what you would like to see happen with errors like the member-ordering ones in my image above (maybe the answer is turn off those rules)

[colingm]

@shellscape checking back in, anything I can do to help out with upgrading @typescript-eslint? I was able to upgrade locally and get lint to pass (had to move some constructors)

[shellscape]

I ran into a major hurdle with my editor (Atom. I know, I know) and the linter plugins and ESLint v8 that I'm still trying to resolve. Planning on getting back to that soon.

[shellscape]

OK got everything working. Now, it gets dicey how this has to happen. Because of the hoisting of ESLint, we need to update the dependency only on the eslint rollup plugin in packages, and then update the eslint-config-rollup package in the root, then fix all of the errors (the import plugin is especially heinous as there were some breaking changes with how it views *.d.ts files without an accompanying .js file), and then push that to master. Once that's done, we need to rebase your fork+branch off of upstream/master. It's bananas but this eslint plugin is a special case. I should have some time in the next few days to make all that happen and then we can get this moving foward.

[colingm]

@shellscape I know you are probably quite busy so I don't want to bother you too much, but let me know if there is any step of the above process I can help with to make it smoother, would love to unblock this

[shellscape]

I still have this on my radar, not to worry

[colingm]

@shellscape anything I can do to help out?

[shellscape]

Thanks for checking in. I haven't lost track of this yet, not to worry.

[lukastaegert]

Hi! I just created a bunch of PRs to update all plugins for Rollup 3. These will be major version bumps as we explicitly drop Node < 14 compatibility. It would be nice if we could merge the ESLint PR with yours. It so happens that the repo ESLint issues should be solved now as well as I also updated the Repo-wide ESLint version in the process.

I also wanted to update all plugin dependencies but actually happened to give up on this one 😱

Suggestion:

• Can you rebase on the branch repo/rollup-3-updates and merge the changes from #1296 into your PR?
• Change the PR target to the branch repo/rollup-3-updates instead of master
• Make sure to list both your breaking change and Node 14 compatibility in the BREAKING CHANGES of the commit message
• Adapt the PR title to include "prepare for Rollup 3" so that I do not overlook it?

If you do that, then I will close #1296 and merge your PR instead together with the other PRs in the next days.

[colingm]

@lukastaegert i will go ahead and take care of all that on Monday morning EST, thank you!

[colingm]

@lukastaegert I have gone ahead and done all of the requested rebases and merges, let me know if everything looks alright or if there are any other requested changes.

[colingm]

@lukastaegert it looks like this auto closed since you deleted the base branch maybe, is there something else you need me to do here?

[lukastaegert]

Damn it, I hoped it would just change your target branch to master as I merged the original branch into master. Can you somehow reopen the PR and point it to master instead?

Apparently, it worked for all other PRs pointing to that branch except yours, maybe this does not work for PRs from forks.

[colingm]

yeah, I will rebase real quick but I will probably need to open a new PR since it won't let me change base on this one

[colingm]

@lukastaegert https://github.com/rollup/plugins/pull/1309

[colingm]

Github doesn't like it when you delete target branches for open PRs, it chokes and dies

[lukastaegert]

Thanks so much

[lukastaegert]

Ok, auto-publish did not work, probably because refactor does not trigger releases. Will sort this out later.