microsoft / microsoft/AzureStorageExplorer
Add lint rule enforcing type imports in a separate block before other imports
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:
- Type-only imports are grouped into their own contiguous block.
- That block appears before all value imports.
Likely implementation, to be confirmed by whoever picks this up:
@typescript-eslint/consistent-type-importsto ensure type-only imports actually useimport typesyntax, so they are statically distinguishable.import/order(or the equivalent already in use) with an explicittypegroup 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/Standaloneand every package undersrc/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.tsfiles undersrc/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
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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