Test case for composition order

[plesiecki]

Description

One more test case for composition order. An expected order would be mc_folder2 before mc_foo.

Follow-up of https://github.com/tivac/modular-css/pull/890

Motivation and Context

Consistent composition map.

How Has This Been Tested?

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [ ] My code follows the code style of this project.
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [ ] I have read the CONTRIBUTING document.
• [ ] I have added tests to cover my changes.
• [ ] All new and existing tests passed.

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 35c203841d01dc5d08689f7362e32c1c8feb68d2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages

Name	Type
@modular-css/browserify	Patch
@modular-css/processor	Patch
@modular-css/svelte	Patch
@modular-css/cli	Patch
@modular-css/css-to-js	Patch
@modular-css/glob	Patch
@modular-css/postcss	Patch
@modular-css/rollup	Patch
@modular-css/vite	Patch
@modular-css/webpack	Patch

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

[ghost]

👇 Click on the image for a new way to code review

• Make big changes easier — review code in small groups of related files

• Know where to start — see the whole change at a glance

• Take a code tour — explore the change with an interactive tour

• Make comments and review — all fully sync’ed with github

  Try it now!

Legend

[netlify[bot]]

✅ Deploy Preview for m-css ready!

Name	Link
🔨 Latest commit	35c203841d01dc5d08689f7362e32c1c8feb68d2
🔍 Latest deploy log	https://app.netlify.com/sites/m-css/deploys/638466b55402d7000901a848
😎 Deploy Preview	https://deploy-preview-892--m-css.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[codecov[bot]]

Codecov Report

Base: 99.71% // Head: 99.71% // Decreases project coverage by -0.00% :warning:

Coverage data is based on head (d4284e9) compared to base (d937088). Patch coverage: 100.00% of modified lines in pull request are covered.

:exclamation: Current head d4284e9 differs from pull request most recent head 35c2038. Consider uploading reports for the commit 35c2038 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #892      +/-   ##
==========================================
- Coverage   99.71%   99.71%   -0.01%     
==========================================
  Files          37       37              
  Lines        1423     1422       -1     
  Branches      232      232              
==========================================
- Hits         1419     1418       -1     
  Misses          4        4              

Impacted Files	Coverage Δ
packages/processor/processor.js	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[tivac]

Sorry, missed the notification on this one @plesiecki, I'll try to get some eyes on it tonight ❤

[plesiecki]

Hello, just to let you know that with 50220c1b80c41c8757c56e3727bd718b95a0370b (instead of d472863dae649efcfe1de039facf69a233209182) I see consistent results for each case. Maybe it's worth reconsidering.

edit: this idea is no longer valid

[plesiecki]

I've added few more tests for composition

[plesiecki]

Looks like a removing this line below can help https://github.com/tivac/modular-css/blob/e64362e087f4ae6113a1b622e59dab827a9f2e97/packages/processor/processor.js#L513

[plesiecki]

PASS packages/processor/test/composition.test.js; rest of the snapshots likely should be regenerated.

[tivac]

The problem with https://github.com/tivac/modular-css/commit/50220c1b80c41c8757c56e3727bd718b95a0370b was, as I recall, that it outputs the composition selectors in the wrong order. Since we're already building the dependency graph including all the selectors, I think it probably needs to start generating composition data from that instead of just shoving it onto an array?

I haven't looked at this for a bit despite saying I would, I'm sorry.

[plesiecki]

The problem with 50220c1 was, as I recall, that it outputs the composition selectors in the wrong order. Since we're already building the dependency graph including all the selectors, I think it probably needs to start generating composition data from that instead of just shoving it onto an array?

I haven't looked at this for a bit despite saying I would, I'm sorry.

No problem. Forget about https://github.com/tivac/modular-css/commit/50220c1b80c41c8757c56e3727bd718b95a0370b, I've approached it differently — https://github.com/tivac/modular-css/pull/892/commits/43c126a57ca34f7d064dd803e32d204999199502 and I'd like to know your opinion on this.

[tivac]

No problem. Forget about 50220c1, I've approached it differently — 43c126a and I'd like to know your opinion on this.

I need to spend some time really understanding why the fix is to remove a dependency that, at first glance, seems to be correct. The snapshots I spot-checked all looked right so I'm confident it's doing the right thing, I just need to know why. I haven't forgotten about this, just need to claw some time back to poke at it further to make sure I understand.

[tivac]

Thanks again for tracking this down @plesiecki, I finally think I have a handle on why the fix works and this is released as @modular-css/[email protected].