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

feat: rewrite to follow other standard engines usage #254

Merged
merged 5 commits into from
Oct 3, 2022

Conversation

theoludwig
Copy link
Contributor

@theoludwig theoludwig commented Jul 23, 2022

I found some free time to rewrite ts-standard.

Fixes #102
Fixes #204
Fixes #209
Fixes #216
Fixes #217
Fixes #171
Fixes #222

This PR is a major BREAKING CHANGE, and involves trade-offs and probably discussions about what features we want to have and how these changes are impacting ts-standard users. I will try to explain as much as possible choices and implementation details, feel free to disagree on certain points, and debate about how we want the future of ts-standard to look.

As the diff is quite large, it might be easier to review it directly file by file entirely first.

This PR is not complete but is ready for review and discussion, the changelog is missing, and we should write a easy to follow guide for our current users to migrate to this new version.
We certainly should do first a prerelease of this version before making it stable, to ensure it works as we want.

Why the rewrite of ts-standard?

As discussed in #171 (comment), ts-standard codebase is so different from other standard engines like standard, semistandard, standardx etc, that it is hard to keep up with changes.
When ESlint v8 was released, we had a hard time upgrading the whole standard ecosystem because ESLint v8 drop the old CLIEngine programmatic usage API, so we had to do a major refactor of standard-engine, as we did for standard-engine@15.

ts-standard currently redefines its own standard-engine, adds more CLI flags and the usage is different from standard.
We do this rewrite to ease the maintenance of ts-standard and keep the usage similar to other standard engines.

This PR also add support for ESLint v8, update eslint-config-standard-with-typescript to v22, and fixes many issues with recent version of TypeScript and ESLint with TypeScript. 😄

Drawbacks/BREAKING CHANGE

We use the same options and CLI flags as standard-engine except --project to configure a tsconfig.json path.
That means options in package.json in ts-standard property like files and report are removed, if we want to add support for them, we'll implement them in standard-engine directly so the other standard engines benefit from them too, btw about the --report option, there is already an issue for it: standard/standard-engine#174.

@theoludwig theoludwig added the enhancement New feature or request label Jul 23, 2022
@theoludwig theoludwig self-assigned this Jul 23, 2022
CHANGELOG.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
constants.js Show resolved Hide resolved
resolve-tsconfig.js Show resolved Hide resolved
Copy link
Member

@mightyiam mightyiam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this, @divlo. Reviewing it does not align with my priorities. I trust it will be figured out without me.

@lindluni
Copy link
Contributor

lindluni commented Oct 2, 2022

Hi all 👋🏻

Do we think these changes will be reviewed in the next couple of weeks? We need to update other dependencies of the https://github.com/github/super-linter project but this is holding us back due to the nature of our project. We are trying to determine if we should drop support for ts-standard so we can bump other deps or if we should continue to wait.

Thank you for these changes and hopefully we see them land soon 😃

@theoludwig
Copy link
Contributor Author

Hey @lindluni! 👋

I don't think, these changes will be reviewed soon, unfortunately.
Meanwhile, you can use ESLint with eslint-config-standard-with-typescript instead of ts-standard.

Copy link
Member

@LinusU LinusU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the long time getting a review on this 🙈

This looks great! Ship it 🚀

@theoludwig theoludwig changed the title BREAKING CHANGE: Rewrite ts-standard to follow other standard engines usage feat: rewrite to follow other standard engines usage Oct 3, 2022
@theoludwig theoludwig merged commit c3de7fe into master Oct 3, 2022
@theoludwig theoludwig deleted the rewrite-support-eslint-v8 branch October 3, 2022 11:37
@theoludwig
Copy link
Contributor Author

Thanks for the review. 😄
Opened a new issue for releasing v12.0.0 (I don't have the right on npm), see: #262.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
4 participants