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

New: extensions property in config #48

Closed
wants to merge 1 commit into from
Closed

New: extensions property in config #48

wants to merge 1 commit into from

Conversation

g-plane
Copy link
Member

@g-plane g-plane commented Nov 12, 2019

View Markdown

Summary

This RFC proposes adding one property called extensions at top level of ESLint configuration files.

Related Issues

Copy link
Sponsor

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

What about adding support for a more generic place for otherwise-CLI options, and finding a way (even if that means a backport) to make "having that object" not break older eslints?


```json
{
"extensions": ["ts", "vue"]
Copy link
Sponsor

Choose a reason for hiding this comment

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

given that unknown properties results in an error, wouldn't that mean that adding this config setting to a shared config is a breaking change?

imo, features for shared configs that are breaking changes are not truly for shared configs :-/


## Backwards Compatibility Analysis

- Adding the new `extensions` property at top level of configuration files will change the schema of configuration, which is a breaking change. Users using old version ESLint can't use this feature because once they add this property, configuration schema check would fail.
Copy link
Sponsor

Choose a reason for hiding this comment

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

this is an argument against that schema check itself :-/

@mysticatea
Copy link
Member

Hi. I have a question.

What is the advantage of this RFC over #20?

#20 has been approved and it solves eslint/eslint#10828. Why do you want another way to solve the issue that has been solved?

@mysticatea mysticatea added Initial Commenting This RFC is in the initial feedback stage and removed triage labels Nov 12, 2019
@g-plane
Copy link
Member Author

g-plane commented Nov 12, 2019

Didn't you ask me to send an RFC? Or did I misunderstand anything?

@mysticatea
Copy link
Member

Didn't you ask me to send an RFC?

If you want to add the extensions top-level property, you need to send an RFC because it's a core feature that has not been accepted. But there is an accepted RFC (#20) already.

Therefore, my previous comment was

However, this direction is different a bit from the current consensus. As mentioned in #10828 (comment), we have RFC20 and RFC22 as approved approach. If you want to support the extentions top-level property, would you send an RFC similar to RFC22?
eslint/eslint#12555 (comment)

If you really want to add the extensions top-level property, I thought that you have the reason that the extensions property is better than #20. So, if you sent the RFC, I had intended to see the reason in the "Alternatives" section of the RFC. But because I didn't see the reason in this RFC, my next question was "what is the advantage of this RFC over #20?"

@g-plane
Copy link
Member Author

g-plane commented Nov 12, 2019

I'm just going to implement the original issue, and the PR is still in progress so it's different from RFC 20. I can't understand why I need to create a new RFC for that feature.

@mysticatea
Copy link
Member

The issue has the accepted label because #20 has been approved. We have not accepted the original proposal as-is. If you implement #20, you don't need any RFC. If you want to implement the extensions property, you need an RFC to discuss the change of our direction.

@g-plane
Copy link
Member Author

g-plane commented Nov 12, 2019

Are this issue and RFC 20 related? Or what's their relationship?

@mysticatea
Copy link
Member

Both the original issue and #20 is the specification for the way people add additional kind of lint targets by config files.

@g-plane
Copy link
Member Author

g-plane commented Nov 12, 2019

Hmm...sounds reasonable. I think I did something wrong because of misunderstanding that issue and #20, and I'm going to close. Sorry for wasting your time.

@g-plane g-plane closed this Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Initial Commenting This RFC is in the initial feedback stage
Projects
None yet
3 participants