Skip to content

Loosen @typescript-eslint/naming-convention rule to allow more formats for import names#388

Merged
Mrtenz merged 1 commit intomainfrom
mrtenz/allow-more-import-casings
Feb 19, 2025
Merged

Loosen @typescript-eslint/naming-convention rule to allow more formats for import names#388
Mrtenz merged 1 commit intomainfrom
mrtenz/allow-more-import-casings

Conversation

@Mrtenz
Copy link
Copy Markdown
Member

@Mrtenz Mrtenz commented Feb 19, 2025

The bump to a more recent version of the TypeScript ESLint plugin added support for specifying naming conventions for import names (import name from 'package';). We didn't add a case for this, so the default format of camelCase applied, meaning that this is not allowed:

import SomeClass from 'some-package';

I've updated the rule to allow camelCase, PascalCase, snake_case, and UPPER_CASE, since the names of imports can depend on the (3rd party) package, and it feels unnecessary to enforce more strict formats in this case.

@Mrtenz Mrtenz marked this pull request as ready for review February 19, 2025 13:13
@Mrtenz Mrtenz requested review from a team as code owners February 19, 2025 13:13
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

Note that we could still alias the imports, so we're not beholden to package authors completely here.

But still, loosening this seems harmless to me, LGTM

@Mrtenz Mrtenz force-pushed the mrtenz/allow-more-import-casings branch from d6b10ac to 0fa8bf3 Compare February 19, 2025 14:35
@Mrtenz
Copy link
Copy Markdown
Member Author

Mrtenz commented Feb 19, 2025

Note that we could still alias the imports, so we're not beholden to package authors completely here.

True, but in the case of class imports for example, we'd want to use PascalCase, which previously wasn't allowed.

@Mrtenz Mrtenz merged commit 3302ab8 into main Feb 19, 2025
22 checks passed
@Mrtenz Mrtenz deleted the mrtenz/allow-more-import-casings branch February 19, 2025 14:40
Gudahtt added a commit to MetaMask/metamask-extension that referenced this pull request Jul 3, 2025
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
Note: This PR is an update version from
#32021 due to
loosened eslint rules. Discussion can be found
#32021 (comment).

This PR addresses TypeScript ESLint error
`@typescript-eslint/naming-convention` across the repository following
the planned upgrade to `@metamask/eslint-config` v14.x.0(release notes
[here](https://github.com/MetaMask/eslint-config/releases?page=1)). For
`@typescript-eslint/naming-convention` violations:
- Use latest `@metamask/eslint-config` rule from
MetaMask/eslint-config#388
- Added eslint-disable directives where proper typing would require
significant refactoring
- Each disable directive includes a TODO comment pointing to the
tracking issue #31860

This approach follows our strategy for handling ESLint upgrades. The
comprehensive fixes for these typing issues will be addressed in
dedicated PRs to avoid blocking development.

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/32106?quickstart=1)

## **Related issues**

Fixes: #23528

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
Co-authored-by: Mark Stacey <mark.stacey@consensys.net>
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.

2 participants