Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request: backporting flat config bug fixes to v8 #17966

Closed
bradzacher opened this issue Jan 6, 2024 · 28 comments
Closed

Request: backporting flat config bug fixes to v8 #17966

bradzacher opened this issue Jan 6, 2024 · 28 comments
Assignees
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly

Comments

@bradzacher
Copy link
Contributor

bradzacher commented Jan 6, 2024

As requested by @fasttime - filing this as a new issue.

What did you expect to happen?

Flat Configs should not crash on v8 of ESLint

What actually happened?

Flat Configs can crash on v8.

Additional comments

There are some flat comfig issue like #17820 who have had their fixes merged after the v9 alpha release was prepared.

This unfortunately means that their fixes will not ship with v8 and instead will have to wait to ship with v9. This is a problem because in v9 flat configs are a breaking change, hard requirement.

Users that hit bugs with flat configs in v8 will be unable to migrate their config ahead of time and instead will be forced to try and manage the config upgrade as well as all of the other breaking changes that will be introduced in v9.

This is a problem because it greatly increases the workload to action the major bump. It also increases the risk involved in upgrading because there are more changes that have to be actioned. Together these can turn many users away from actioning a major upgrade - especially at a company where work needs to be scheduled or prioritised it can be really hard to get a big, risky piece of work on the roadmap when it has small returns (v9 likely won't add much immediate value for users as a lot of the changes will be internal).

In addition to the problem for users - there is also the same problem for ecosystem partners. For example the @typescript-eslint is currently hard-blocked by #17820 - it crashes our lint setup and is preventing us from upgrading ourselves. This is a major issue because if an ecosystem partner can't upgrade - they can't build support. And indeed not being able to upgrade our repo means we can't dogfood our flat config support - we can't reasonably ship something to users we haven't tested or that we know is known to be broken.

We'd love to ship support for flat configs now (typescript-eslint/typescript-eslint#7935) but without a fix we'll have to wait for v9 to ship support. This is especially scary for us because given our tight integration with ESLint we know there are going to be other breaking changes we need to handle. As a project we'll be looking to support v8 and v9 simultaneously - so we will have a lot of work to build and test for both versions at the same time. Anything we can do to reduce risk by shipping now would be a huge deal.


So after that wall of text.. What's the ask? We ask that you backport bug fixes to v8 to help people migrate ahead of the v9 release.

By ensuring flat configs in v8 are crash-free it enables users and ecosystem partners to upgrade ahead of v9, which is a big win for everyone.

@nzakas
Copy link
Member

nzakas commented Jan 11, 2024

Unfortunately, this is just not something we can do in any reasonably safe way. Our whole release process is based on releasing just one thing and we have never backported fixes to previous versions. We literally don't have the automation to release anything other than HEAD, so we'd have to do it manually, which would be a ton of work due to the scope of the changes in v9 (not to mention tie-ins with documentation, the website, etc.).

Maybe after the final v9 is released someone can investigate how to do this, but it's definitely not something we can do quickly or easily.

@woutermont

This comment was marked as off-topic.

@bradzacher
Copy link
Contributor Author

I do understand that it's not going to be easy and I appreciate the scope of what I'm asking for here.

Considering this is hard blocking us, we could unblock ourselves by releasing our own temporary fork of ESLint to backport the fix so that we can unblock ourselves and our users until the v9 release. We'd use this to work on flat config on our end and once v9 is stable we could remove it. Do you see any other potential solutions?

@nzakas
Copy link
Member

nzakas commented Jan 15, 2024

Considering this is hard blocking us, we could unblock ourselves by releasing our own temporary fork of ESLint to backport the fix so that we can unblock ourselves and our users until the v9 release. We'd use this to work on flat config on our end and once v9 is stable we could remove it.

Please don't do that. As soon as you create a fork, then people will notice and start encouraging others to use it for the same reason. Then we have fragmentation.

I'd say the best use of your time at this point would be to identify the commits that you'd like to see backported into v8.x and post them here.

As I said, we're just not going to have the cycles to dig into this on our end until after v9 is released, so any help you're willing to give would be greatly appreciated.

@nzakas nzakas added the tsc agenda This issue will be discussed by ESLint's TSC at the next meeting label Jan 15, 2024
@nzakas
Copy link
Member

nzakas commented Jan 15, 2024

TSC Summary: We've made several bug fixes to flat config in the v9 branch that aren't in the v8 branch, which makes it difficult for plugins to support both v8 and v9 at the same time. This issue requests backporting those changes into v8 and then doing one last release in v8. At the moment, we don't have a way to release from any branch other than main, and we generate other artifacts (docs, release notes, blog posts) that all assume on branch.

TSC Question: Do we want to backport flat config fixes into v8? If so, how can we go about this in the least disruptive way possible?

@bradzacher
Copy link
Contributor Author

bradzacher commented Jan 15, 2024

I'd say the best use of your time at this point would be to identify the commits that you'd like to see backported into v8.x and post them here.

The current main blocker for us so far is the commit for the issue mentioned in the OP - #17906

There may be others we find as we continue to iterate - but currently the lint setup in our repo hard crashes once converted to flat configs due to this issue - so it's hard to iterate further at this time.

As soon as you create a fork, then people will notice and start encouraging others to use it for the same reason. Then we have fragmentation.

I definitely agree - which is why we mention it as a temporary, last resort. Realistically we have a few options:

  1. the fixes are backported to v8 and released, officially
  2. we fork v8, backport the fixes and release the fork as a temporary workaround for those impacted with the intention aggressively deprecate it once v9 releases
  3. we do nothing, wait until v9 releases, and then work on building support for v9

However (3) isn't an option. Given we power ~70% of ESLint users - there's an expectation that we support ESLint majors close to releases. Missing support means we hold back the ecosystem from progressing. For example right now we don't officially support flat configs and so a lot of people aren't even considering them - they'd rather wait for us to release our shareable flat configs.
Thus we HAVE to build support for v9 before v9 releases - or else we face maintenance burden as people request support, and stress as we push to release it in a tight timeline.

So the only options are either (1) or (2).
I don't want to come across as "blackmailing" when I say this - but ultimately the if fixes aren't able to be backported officially; then we'll have to backport them in a temporary fork.

@nzakas
Copy link
Member

nzakas commented Jan 16, 2024

So the only options are either (1) or (2). I don't want to come across as "blackmailing" when I say this - but ultimately the if fixes aren't able to be backported officially; then we'll have to backport them in a temporary fork.

Do what you need to do. I don't have anything additional to add to this topic at this time.

@nzakas
Copy link
Member

nzakas commented Jan 17, 2024

Going through the flat config changes in the v9 releases. Here are the candidates for backporting:

There were some other changes, but they would be breaking, so I think these are the ones to consider.

@woutermont
Copy link

@nzakas, not sure why you referred me to here. My comment in #17820 was specifically targeted at @bradzacher for circumventing that issue in typescript-eslint. While it might alleviate the need for a backport of that specific fix, it has nothing to do with backporting in general.

@nzakas
Copy link
Member

nzakas commented Jan 17, 2024

@woutermont sorry, you were commenting on a thread that led to this issue, so I invited you here to continue. If that's not the topic you care about, I'd suggest opening a different discussion so as not to confuse ongoing conversations.

@nzakas
Copy link
Member

nzakas commented Jan 23, 2024

I've been digging into the hairy details of how a backport might work, and here's what I've found so far. All of this is assuming we have a v8.x branch that contains the code for the v8.x line. We were already planning on creating this to preserve the v8.x docs, so that part is a given. (This is also slightly complicated by #17965 (comment) -- something to be aware of.)

  1. Most of our automation works with HEAD, which is the same as main in most cases. But that also means if we check out a different branch, everything mostly just works. This includes: package.json update, npm publish, push back to GitHub, generating GitHub release, generating CHANGELOG.md, and generating the docs site. All of these are self-contained in the HEAD branch.
  2. We would need to update the Jenkins release job to allow us to select the branch to check out from the eslint repo.
  3. We would need to prevent the docs site from being pushed to latest to prevent overwriting the current docs site (for v9.x). The quick and dirty way would be to run git branch --show-current and if it's not main then assume a backport.
  4. The big issue is that, once released with our usual process, the backported version will be tagged as latest on npm. I'm not sure if it's possible to publish without a tag, but I've not come across any details on how to do that. We could YOLO it and do a backport release immediately followed by a main release, in which case the latest package will only momentarily be the backport version, or we could publish to some other tag. (Note: The homepage of eslint.org fetches data from npm to display the most recent version and next version on the homepage. After doing a backport, this would end up displaying the backport version if it were latest.)

@bradzacher
Copy link
Contributor Author

For 4 - npm will default to the latest tag - but it can be overridden with any other tag.

You could explicitly tag the release as backport and then remove the tag if you want to clean up afterward.

Eg like npm publish --tag backport && npm dist-tag rm backport

@nzakas
Copy link
Member

nzakas commented Jan 24, 2024

Perhaps an easier way: if we do a new release from a v8.x branch before v9.0.0 is released, I believe everything should just work in relation to the latest branch and npm tag.

@nzakas nzakas added the accepted There is consensus among the team that this change meets the criteria for inclusion label Jan 25, 2024
@bmish
Copy link
Sponsor Member

bmish commented Jan 25, 2024

For the record, I am in favor of backporting, while recognizing the extra work needed by the team to pull it off.

Beyond just these particular flat config bug fixes, I suspect there could occasionally be other bug fixes or even smaller features worth considering for backporting, to make multi-version compatibility easier for the ecosystem and perhaps occassionally to enable users stuck on old major versions to gain access to key features (like Node itself does).

@nzakas
Copy link
Member

nzakas commented Jan 25, 2024

This issue is about backporting just these fixes, just this time. We aren't entertaining ongoing backports of bugs going forward.

@sam3k
Copy link
Contributor

sam3k commented Jan 27, 2024

Per 2024-01-25 tsc-meeting, we've decided to:

  • Do a backport release for the flat config changes
  • The backport v8.x release will happen prior to the final v9.0.0 release
  • We've agreed to publish to the latest tag
  • The branch should be named v8.x

@sam3k sam3k removed the tsc agenda This issue will be discussed by ESLint's TSC at the next meeting label Jan 27, 2024
@mdjermanovic
Copy link
Member

Thoughts about backporting #17989 as well?

@snitin315
Copy link
Contributor

Thoughts about backporting #17989 as well?

I believe we can backport this too.

@nzakas
Copy link
Member

nzakas commented Jan 29, 2024

#17989 changes RuleTester in v9, which was FlatRuleTester in v8. So would that mean we are updating FlatRuleTester in v8? If so, that would need to be done manually due to the file renaming.

@mdjermanovic
Copy link
Member

#17989 changes RuleTester in v9, which was FlatRuleTester in v8. So would that mean we are updating FlatRuleTester in v8? If so, that would need to be done manually due to the file renaming.

Yes, in v8.x we would update flat-rule-tester.js, and this would need to be done manually because the original change is in a different file.

@snitin315
Copy link
Contributor

Yes, in v8.x we would update flat-rule-tester.js, and this would need to be done manually because the original change is in a different file.

Yes, I'll do it.

@nzakas
Copy link
Member

nzakas commented Feb 2, 2024

In relation to backporting, the VS Code folks are saying that the current setup of /use-at-your-own-risk makes it difficult to support flat config by default. They're requesting an additional API to be available in v8, v9, and v10 to make it easier.

VS Code plugin issue: microsoft/vscode-eslint#1644

@mdjermanovic
Copy link
Member

In ESLint v8.57.0, as a side effect of backporting support for eslint.config.mjs and eslint.config.cjs config file names, if those files are present, shouldUseFlatConfig() will return true and CLI will automatically switch to flat config mode. Is that okay?

@snitin315
Copy link
Contributor

I believe that is ok, it should be returning true already if eslint.config.js is present.

@nzakas
Copy link
Member

nzakas commented Feb 6, 2024

Agreed, that is okay and I think is the expected behavior.

@mdjermanovic
Copy link
Member

ESLint v8.57.0 has been released:

https://eslint.org/blog/2024/02/eslint-v8.57.0-released/

@mdjermanovic
Copy link
Member

All planned tasks (#17966 (comment)) have been completed, so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly
Projects
Archived in project
Development

No branches or pull requests

7 participants