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
Change extends
in overrides
to be consistent with plugins
behaviour
#6380
Change extends
in overrides
to be consistent with plugins
behaviour
#6380
Conversation
🦋 Changeset detectedLatest commit: 01cd74e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
test output:
|
@jasikpark Thanks for opening this pull request to show the current behaviour.
I've been looking back through some issues (#5603, #5656 etc) and it doesn't look like we ever discussed what the intended behaviour should be. It feels safe to merge Although the current behaviour is undocumented, we can't know how many users rely on it. Anyone else got thoughts on this? Whatever we decide, let's update our docs to describe the behaviour for each of the most used configuration properties: At the moment, I believe it's:
|
extends
in overrides
@jeddy3 Thanks for summarizing the issues.
I agree with the idea that |
OK, let's make the @jasikpark Feel free to fix the behaviour in this pull request and update the docs. For the docs, we can add a bullet at the end of this section.
|
7ea9f8a
to
d471842
Compare
Okay, pushed some basics docs & merging |
looks like |
extends
in overrides
extends
and other complex config properties in overrides
I guess everything in stylelint/types/stylelint/index.d.ts Lines 36 to 57 in cf04b2b
that is an array should be merged right? since technically any of those can be passed to overrides? |
might be worth looking to eslint's behavior here as well https://eslint.org/docs/latest/user-guide/configuring/configuration-files#how-do-overrides-work |
Seems that
I'd suggest maybe this PR can just alter |
Since I'm not sure what the semantics are of all these config values |
extends
and other complex config properties in overrides
extends
in overrides
to be consistent with plugins
behaviour
@jasikpark Thanks for making the changes and for digging more into the overrideable options.
That sounds best. The general behaviour you outline in #6380 (comment), SGTM. We're going to remove Processors in the next major release. That leaves the following as user-facing options:
If we haven't already, it'd be good to have tests for these. I've added a changeset entry. I think this pull request is ready for review. |
I suppose my documentation update is inaccurate now if I'm only going to alter Maybe I should add tests + docs that describe where we are today && then another PR can fix everything? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I should add tests + docs that describe where we are today && then another PR can fix everything?
Would you mind doing that in another pull request?
I've made a suggestion to tweak the docs for this pull request.
extends
in overrides
to be consistent with plugins
behaviourextends
in overrides
to be consistent with plugins
behaviour
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM 👍🏼
This is an incremental commit, I think I need an explicit list of what should get merged how
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
ed62a9c
to
1e176e4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
We can merge once we start actively working on a v15
branch.
I've created a tracking issue for preparing 15.0.0 (#6409).
I've updated the PR base branch to |
I've moved the migrating information out of the changeset and into a migration guide doc.
Yes. |
This merge behaviour is aligned with how a flat config would work, and so is a step in the right direction for us.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jeddy3 Thanks so much for writing up the migration guide. There may be a few typos left, but LGTM! 👍🏼
thx for getting this merge ready 💜 |
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
We can merge this when CI passes. 💪🏼 |
…iour (#6380) Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com> Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
overrides
extends
arrays together inoverrides
extends
merging functionality