Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[no-named-as-default] Add ignorePaths option #3131

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Sahar541998
Copy link

@Sahar541998 Sahar541998 commented Dec 26, 2024

Adding Ignore Paths Option to no-named-as-default Rule

Summary

This PR introduces an ignorePaths option to the no-named-as-default ESLint rule.

The purpose of this enhancement is to provide flexibility in cases where certain packages, like styled-components, require importing a named export as a default export. These imports are valid but may be incorrectly flagged by the current rule.

Use Case

For example, the following import is valid in the context of styled-components, but it may be flagged by the no-named-as-default rule:

import styled from 'styled-components';

With the new ignorePaths option, you can specify patterns to exclude such packages or paths from this rule's validation.

Changes

Added ignorePaths configuration to the no-named-as-default rule.
Updated rule documentation to reflect the new option and its usage.

Configuration Example

To use this new option, include it in your ESLint configuration file:

module.exports = {
  rules: {
    'import/no-named-as-default': [
      'error',
      {
        ignorePaths: ['styled-components', 'some-other-package']
      }
    ]
  }
};

Copy link

codecov bot commented Dec 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.62%. Comparing base (fa36d49) to head (0e5a4a7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3131      +/-   ##
==========================================
+ Coverage   95.47%   95.62%   +0.14%     
==========================================
  Files          83       83              
  Lines        3628     3632       +4     
  Branches     1281     1284       +3     
==========================================
+ Hits         3464     3473       +9     
+ Misses        164      159       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ljharb
Copy link
Member

ljharb commented Jan 29, 2025

Can you help me understand why an option is needed here, instead of using overrides to disable the rule for whatever files you want?

@Sahar541998
Copy link
Author

Sahar541998 commented Jan 29, 2025

Can you help me understand why an option is needed here, instead of using overrides to disable the rule for whatever files you want?

For my usage this library (styled-component) is used almost across all my app

And i still want to use this rule because thats my project convention
But i still want to use the 3rd party convention and call it "styled"

@ljharb
Copy link
Member

ljharb commented Jan 29, 2025

Can you help me understand the convention? What is the named "styled" export from styled-components, and why does it have the same name as the default in practice?

@Sahar541998
Copy link
Author

Can you help me understand the convention? What is the named "styled" export from styled-components, and why does it have the same name as the default in practice?

Yes, sure! 😊

I'm using the styled-components library, which is a popular styling library for React.

It exports a styled function that allows you to define CSS for HTML elements. For example, to style a title, you would use:

import styled from 'styled-components';

const Title = styled.h1`
  text-align: center;
  color: blue;
`;

By convention, the library uses styled as the function name, which is why you'll see this import in almost every file of a React project using styled-components.

I want to keep using this convention so that new developers can easily understand and work with my code.

To avoid adding lint ignores for every usage, my current workaround is to set this ESLint rule as a warning instead of an error.

@Sahar541998
Copy link
Author

Sahar541998 commented Jan 29, 2025

Additionally, my intention in adding this option is that other libraries might follow a similar pattern, making it useful for others as well.

@ljharb
Copy link
Member

ljharb commented Jan 29, 2025

Right, but if this is what everyone does, what's the import { styled } for? Why is it still there?

@Sahar541998
Copy link
Author

Right, but if this is what everyone does, what's the import { styled } for? Why is it still there?

Valid point, Not sure why its there if in the docs they are using the default
But both are valid

@ljharb
Copy link
Member

ljharb commented Jan 29, 2025

… and the point of this rule is, when both exist, you should prefer the name. In your own codebase, do you have both exist ever? if so, why?

@Sahar541998
Copy link
Author

… and the point of this rule is, when both exist, you should prefer the name. In your own codebase, do you have both exist ever? if so, why?

Nope in my code i never have both
Just in this package

@Sahar541998
Copy link
Author

But you probably right
Since this package allow both it's probably the best to only use the named one and not default and ignore the docs convention

So we might not need this addition

@ljharb
Copy link
Member

ljharb commented Jan 29, 2025

Then if you never have both, why do you need the rule enabled at all?

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

Successfully merging this pull request may close these issues.

3 participants