Skip to content

fix: bugs#2086

Merged
lishaduck merged 7 commits intomainfrom
disables
Jan 27, 2026
Merged

fix: bugs#2086
lishaduck merged 7 commits intomainfrom
disables

Conversation

@lishaduck
Copy link
Member

@lishaduck lishaduck commented Jan 26, 2026

PR Checklist

Overview

I haven't actually tracked down #1854 yet, but I found enough other bugs I'm submitting this preliminarily, feel free to merge independently.

@changeset-bot
Copy link

changeset-bot bot commented Jan 26, 2026

⚠️ No Changeset found

Latest commit: 33e7791

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Jan 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
flint Ready Ready Preview, Comment Jan 27, 2026 1:15am

Request Review

Comment on lines +15 to +17
yield chalk.yellow(
`⚠️ Additionally found ${pluralize(diagnostics.length, "diagnostic")}:`,
);
Copy link
Member Author

Choose a reason for hiding this comment

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

We should either make an ESLint rule for this like node-style-text does or switch to Ansis which handles this correctly.

"scripts": {
"build": "tsdown",
"flint": "node --conditions='@flint.fyi/source' packages/flint/bin/index.js",
"flint:cache-ignore": "node --conditions='@flint.fyi/source' packages/flint/bin/index.js --cache-ignore",
Copy link
Member Author

Choose a reason for hiding this comment

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

I was using Zed, and it needed a package.json script to launch debugging correctly.

log(
"Adding %d directives for file %s",
fileMetadata.directives,
fileMetadata.directives.length,
Copy link
Member Author

Choose a reason for hiding this comment

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

Otherwise it logs NaN.

Comment on lines -29 to -36
{
files: "packages/cli/src/**/*",
rules: [
ts.rules({
consoleCalls: false,
}),
],
},
Copy link
Member Author

Choose a reason for hiding this comment

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

@barrymichaeldoyle, you've been working in a tangentially-related area, what are your thoughts on surfacing a Knip-style config hint for this? I think it's annoying that we didn't get a warning but on the other hand, it'd potentially make composing configs really annoying.

Copy link
Contributor

Choose a reason for hiding this comment

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

I personally prefer the idea of the Knip-style config hint 😁 Not sure how annoying it could make composing configs though. If it were up to me, I'd do it and revert later if it's too annoying.

Otherwise RuleCreator'd rule no longer have an id, breaking intellisense & excess property checks for `.rules()`.
>,
): Rule<
RuleAboutWithPresetsAndUrl<Presets>,
// We can't put this in the constraint or else inference fails for some reason.
Copy link
Member Author

Choose a reason for hiding this comment

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

AKA I revived #1418. Not sure what's up with that.

Copy link
Collaborator

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Thanks!

Agreed all around on the threads. They all look good to me as followups.

@JoshuaKGoldberg JoshuaKGoldberg added the ready to merge 2+ team members approved; we're now waiting on the author to file followups and self-merge. label Jan 26, 2026
We never pass this, it isn't real, what isn't real can't hurt me until it starts failing me tests.
@lishaduck lishaduck merged commit cc25ccc into main Jan 27, 2026
8 checks passed
@lishaduck lishaduck deleted the disables branch January 27, 2026 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready to merge 2+ team members approved; we're now waiting on the author to file followups and self-merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants