Skip to content

allow var in TypeScript ambient declarations#68

Merged
cowboyd merged 2 commits into
thefrontside:v3from
azat-io:feat/ts-declarations
Jan 4, 2026
Merged

allow var in TypeScript ambient declarations#68
cowboyd merged 2 commits into
thefrontside:v3from
azat-io:feat/ts-declarations

Conversation

@azat-io

@azat-io azat-io commented Jan 1, 2026

Copy link
Copy Markdown
Contributor

Motivation

The prefer-let rule was incorrectly flagging var declarations in TypeScript ambient contexts (declare global, declare module, declare namespace). In these contexts, var has special semantic meaning - it creates properties on the global object (e.g., window.fathom), while let would create lexical bindings that aren't accessible as properties.

This was forcing developers to add eslint-disable comments for legitimate TypeScript code:

declare global {
  // eslint-disable-next-line prefer-let/prefer-let
  var fathom: undefined | Fathom
}

Approach

Added a new helper function isInAmbientContext() that walks up the AST tree to check if a variable declaration is inside a TSModuleDeclaration with declare: true. When var is detected in an ambient context, the rule now skips it instead of reporting an error.

@changeset-bot

changeset-bot Bot commented Jan 1, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b2b469c

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

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

@github-actions

github-actions Bot commented Jan 1, 2026

Copy link
Copy Markdown
Contributor

Package Changes Through b2b469c

No changes.

Add a change file through the GitHub UI by following this link.


Read about change files or the docs at github.com/jbolda/covector

@cowboyd cowboyd 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.

This looks good to me.

@cowboyd cowboyd merged commit f6167ad into thefrontside:v3 Jan 4, 2026
1 check passed
@azat-io

azat-io commented Jan 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks! Could you publish release?

@cowboyd

cowboyd commented Jan 5, 2026

Copy link
Copy Markdown
Member

@jbolda For some reason, this didn't open up a release PR. Any idea why? https://github.com/thefrontside/javascript/actions/runs/20697051068

GitHub
Frontside JavaScript tooling. Contribute to thefrontside/javascript development by creating an account on GitHub.

@azat-io

azat-io commented Jan 5, 2026

Copy link
Copy Markdown
Contributor Author

@cowboyd Perhaps a changeset should have been added?

@jbolda

jbolda commented Jan 5, 2026

Copy link
Copy Markdown
Member

@cowboyd the workflows have covector which looks for change files in the .change dir. Looks like the changeset bot threw you off, and you added one in .changeset instead? If you move that file, it should trigger the PR open.

@cowboyd

cowboyd commented Jan 5, 2026

Copy link
Copy Markdown
Member

@azat-io looks like I put the changes in the wrong dir! I don't currently have a checkout of this repo, would you mind moving it to the .change and we can get this out the door?

Also, @jbolda we will probably need to set this repo up with OIDC publishing, which I can do now. Were there some changes we needed to make to the publish action itself?

@jbolda

jbolda commented Jan 5, 2026

Copy link
Copy Markdown
Member

@cowboyd just removing the token.

@azat-io

azat-io commented Jan 6, 2026

Copy link
Copy Markdown
Contributor Author

Do I need to do something, or will you publish it?

@cowboyd

cowboyd commented Jan 6, 2026

Copy link
Copy Markdown
Member

Actually, I have some more time today, so I can take care of it.

@cowboyd

cowboyd commented Jan 7, 2026

Copy link
Copy Markdown
Member

This should be available now #69

@azat-io

azat-io commented Jan 7, 2026

Copy link
Copy Markdown
Contributor Author

Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants