microsoft / microsoft/AzureStorageExplorer

Add lint rule enforcing type imports in a separate block before other imports

Open
#9,182 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
No language data
Stars
452
Forks
92
Avg merge
15h 20m
Merged PRs (30d)
3

Description

Problem

Type imports are inconsistently placed throughout the codebase. In some files they are interleaved with value imports; in others they appear below them. There is currently no automated enforcement, so the convention is only applied when a reviewer happens to notice it.

This surfaced during review of !777789 - Fix SDK job type imports in blob/table extensions, where the reviewer asked for type imports to be moved into a separate block above other imports. Investigation showed the placement in question was pre-existing on main, not introduced by that PR -- the change only renamed identifiers on those lines. That makes it a codebase-wide convention gap rather than a defect in any single change, and it means the same review comment will keep recurring on unrelated PRs until it is enforced automatically.

Proposal

Add an ESLint rule to src/components/build-common/eslint.config.mjs enforcing that:

  1. Type-only imports are grouped into their own contiguous block.
  2. That block appears before all value imports.

Likely implementation, to be confirmed by whoever picks this up:

  • @typescript-eslint/consistent-type-imports to ensure type-only imports actually use import type syntax, so they are statically distinguishable.
  • import/order (or the equivalent already in use) with an explicit type group ordered first, to enforce block placement and separation.

Considerations

  • Fleet-wide churn. Enabling this with autofix will touch a large number of files across src/Standalone and every package under src/components. The rollout should be a dedicated PR separate from any feature or bugfix work, so the diff stays reviewable and does not collide with in-flight branches.
  • Sequencing. Several component PRs are in flight right now. This should land during a quiet window, or be staged per-package, to avoid manufacturing merge conflicts across active branches.
  • Version bumps. Any src/components/* package touched by the autofix requires a version bump per repo rules. If the rollout is staged per-package, each stage carries its own bump.
  • Confirm whether the rule should apply to test files and to generated files. Generated *Resources.ts files under src/Standalone/app/resources/ are rewritten by the gulp resource generator on every build, so any lint fix applied to them would be reverted on the next build. Those should almost certainly be excluded, or the generator template updated instead.

Acceptance criteria

  • Lint rule is enabled in the shared ESLint config.
  • Existing violations are fixed, or explicitly and deliberately suppressed with a documented rationale.
  • CI fails on new violations.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read src/components/build-common/eslint.config.mjs and inspect the existing import-related configuration and package lint setup. Determine how the shared rule should cover src/Standalone and src/components, including test files and generated *Resources.ts files. Done means the rule is enabled, existing violations are fixed or documented as deliberate suppressions, and CI rejects new violations.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
tooling
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.