Skip to content

feat: audit "as any as" usage and prefer it over "as unknown as"#144

Merged
frodi-karlsson merged 10 commits into
mainfrom
ban-as-unknown-as
Jun 4, 2026
Merged

feat: audit "as any as" usage and prefer it over "as unknown as"#144
frodi-karlsson merged 10 commits into
mainfrom
ban-as-unknown-as

Conversation

@frodi-karlsson

@frodi-karlsson frodi-karlsson commented Jun 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

tightening the sort of type shortcuts that are available to us

kirillgroshkov
kirillgroshkov reviewed Jun 2, 2026
View reviewed changes
Comment thread packages/dev-lib/cfg/oxlint-plugin-nc.mjs Outdated
Comment thread packages/dev-lib/cfg/oxlint-plugin-nc.mjs Outdated
Comment thread packages/dev-lib/cfg/oxlint-plugin-nc.mjs Outdated
Comment thread packages/js-lib/src/object/keySortedMap.ts Outdated
@frodi-karlsson frodi-karlsson changed the title feat: ban 'as unknown as' in favor of simpler 'any' feat: ban 'as unknown as' in favor of better options, except where we know better Jun 2, 2026
@frodi-karlsson

frodi-karlsson commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Okay, now I feel I have addressed the preference for explicitness

@kirillgroshkov

kirillgroshkov commented Jun 2, 2026

Copy link
Copy Markdown
Member

Okay, now I feel I have addressed the preference for explicitness

But I still see the no-as-x-as rule being enabled/applied. It bans the explicitness of as any as Foo, right? And me and David tried to explain that we like the explicitness.

I like all the changes in the PR, but I still question the need for this rule to exist?

@frodi-karlsson

frodi-karlsson commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

I see!

My point with this PR is that:

  • Having to do as any as is a case of saying "i know better than the compiler, and there's no way to work with it in a reasonable way"
  • Code is increasingly written by actors without common sense, in great quantities. This can mean that alogical behavior can more easily slip through than ever
  • We should urge toward finding better solutions in this case, instead of implicitly saying "yes, just doing as any as is fine"

I think the fact that working around this rule as a first option, rather than disabling the rule on each line, resulted in:

I like all the changes in the PR

is an indicator that the approach can be useful

@frodi-karlsson

frodi-karlsson commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Ah but maybe I don't see.

Are you of the opinion that doing as any as or as unknown as is preferable to solving it with more accurate types?

frodi-karlsson
frodi-karlsson commented Jun 2, 2026
View reviewed changes

@frodi-karlsson frodi-karlsson left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

And places like it

Comment thread packages/db-lib/src/commondao/common.dao.ts Outdated
Comment on lines +279 to +280
if (isPartialQuery) return pipeline as any as Pipeline<BM>
// oxlint-disable-next-line nc/no-as-x-as -- We want to lie here, to avoid the overhead of converting DBM to BM when it's a partial query (since we don't have all the data to create a full BM anyway).
if (isPartialQuery) return pipeline as unknown as Pipeline<BM>

@frodi-karlsson frodi-karlsson Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

And here's where I concede to keeping it, as it's the best way to solve the problems, but the comment just makes it even more explicit

@kirillgroshkov

kirillgroshkov commented Jun 2, 2026

Copy link
Copy Markdown
Member

Ah but maybe I don't see.

Are you of the opinion that doing as any as or as unknown as is preferable to solving it with more accurate types?

No.
Accurate types are best (when it's possible / convenient / ergonomic).
If not - as any as Foo is good/explicit.

I'm not against accurate types, and I like the typing improvements in this PR very much!

What I don't like is when the linter would shout at as any as Foo for violating the no-as-x-as rule. (I question that rule).

But also - I do like your work at adopting typescript/no-explicit-any. Will that rule shout at as any as Foo? Maybe it will. I'm ok to have the as any as Foo suppressed with no-explicit-any, but I don't like when no-as-x-as screams.

Does it make sense?

@kirillgroshkov

kirillgroshkov commented Jun 2, 2026

Copy link
Copy Markdown
Member

Even more TLDR:

I like typescript/no-explicit-any and don't like no-as-x-as.

I like correct types when it's easy, but when it's not - I like as any as Foo [much] more than just as any

@frodi-karlsson

frodi-karlsson commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Alright ✅

@frodi-karlsson

frodi-karlsson commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

TLDR: my argument for the rule is that it's a pattern that should be motivated, which a lint rule enforces (via suppression)

I will revert the rule and change to just the improvements

@frodi-karlsson

frodi-karlsson commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

I see that if we align all to as any as instead of as-unknown-as it's a win-win

@frodi-karlsson frodi-karlsson changed the title feat: ban 'as unknown as' in favor of better options, except where we know better feat: audit "as any as" usage and prefer it over "as unknown as" Jun 2, 2026
kirillgroshkov
kirillgroshkov reviewed Jun 3, 2026
View reviewed changes

@kirillgroshkov kirillgroshkov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, but there's a file I don't understand

Comment thread AppleRootCA-G3.cer Outdated
@frodi-karlsson frodi-karlsson merged commit b6d6b1f into main Jun 4, 2026
3 checks passed
@frodi-karlsson frodi-karlsson deleted the ban-as-unknown-as branch June 4, 2026 06:16
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🎉 This PR is included in version @naturalcycles/dev-lib-v20.49.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🎉 This PR is included in version @naturalcycles/js-lib-v15.78.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🎉 This PR is included in version @naturalcycles/db-lib-v10.54.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🎉 This PR is included in version @naturalcycles/nodejs-lib-v15.111.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🎉 This PR is included in version @naturalcycles/airtable-lib-v4.17.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🎉 This PR is included in version @naturalcycles/mongo-lib-v4.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🎉 This PR is included in version @naturalcycles/sqlite-lib-v2.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ryangibbs ryangibbs ryangibbs left review comments

@kirillgroshkov kirillgroshkov kirillgroshkov approved these changes

@mrnagydavid mrnagydavid Awaiting requested review from mrnagydavid

Assignees

No one assigned

Labels

released

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@frodi-karlsson @kirillgroshkov @ryangibbs @mrnagydavid