--watch mode doesn't always work correctly

[X-Tender]

I have the following postcss config:

{
    "use": [
        "autoprefixer",
        "postcss-import",
        "postcss-simple-vars",
        "postcss-extend",
        "postcss-nested",
        "postcss-mixins",
        "cssnano"
    ],
    "input": "src/scss/*.css",
    "dir": "public/assets/css/",
    "local-plugins": true,
    "watch": true,
    "map": "file",
    "parser" : "postcss-scss",
    "autoprefixer": {
        "browsers": [ "Explorer >= 11", "Edge >= 13", "Firefox ESR", "Opera 37", "safari >= 9", "Chrome >= 50", "ChromeAndroid >= 47", "last 3 iOS versions" ]
    },
    "cssnano": {
        "safe": true,
        "mergeRules": false,
        "browsers": [ "Explorer >= 11", "Edge >= 13", "Firefox ESR", "Opera 37", "safari >= 9", "Chrome >= 50", "ChromeAndroid >= 47", "last 3 iOS versions" ]
    }
}

When I run it the files only get compiled once. When I change it from dir to one css file it works without problems.

"postcss-cli": "^2.6.0",
Window10

[RyanZim]

@X-Tender How many css files do you have in src/scss/?

[X-Tender]

Just two. No includes, just two files for testing.

[RyanZim]

When I change it from dir to one css file it works without problems.

That's weird, postcss should error if you are using a single output file with multiple inputs.

[RyanZim]

I'll try to poke around at this when I get time. @X-Tender if you want to post a test-case repo on GitHub so I can clone it and see the issue for myself, that would be great!

@watilde If you get to this before I do, that's fine.

[RyanZim]

@X-Tender Confirmed Issue. It's not immediately obvious, but the issue is present. Will do a little debugging.

[RyanZim]

OK, tracing this down a bit further, if you remove postcss-import from the plugin list, it works fine.

I suspect global.watchCSS/updateWatchedFiles() is the issue.

Will continue debugging.

[RyanZim]

The problem is (at least partially) in the import hook for postcss-import https://github.com/postcss/postcss-cli/blob/master/index.js#L84.

postcss-cli formerly relied on the (undocumented) fact that inside onImport, this.from was the name of the file that contained the imports. This is no longer so; it is now undefined.

This causes an issue at https://github.com/postcss/postcss-cli/blob/master/index.js#L177. When multiple files use the same null entry point, they overwrite each other in the index, causing elements to be dropped from the watch list.

There is a request at postcss/postcss-import#234 that asks for a reliable way to determine the import parent. If this is resolved, we could then use that to resolve this issue. (I hope that issue is escalated!)

---

The other part of the problem: postcss-cli ought to error if !entryPoint instead of falling back to null. This null behavior works if there is only a single entry point, but silently causes issues with multiple entry points.

If this behavior would have been in place from the beginning, postcss-cli would have errored noisily when postcss-import changed its behavior in regards to this.from and prevented this weird & hard-to-trace bug.

[michael-ciniawsky]

result.messages
  .filter((msg) => msg.type === 'dependency' ? msg : '')
  .forEach((dep) => chokidar.add(dep.file))

[RyanZim]

@michael-ciniawsky That is one option. Does msg.type === 'dependency' work with other @import tools?

[michael-ciniawsky]

Does msg.type === 'dependency' work with other@import` tools?

@RyanZim nope, sry, I was busy with social work the last few weeks and hadn't much time for coding, need to take a look at e.g postcss-smart-import and friends first. Are you one of the maintainers of postcss-cli ? I'm asking because I made a roughly proof-of-concept for postcss-cli with postcss-load-config, watching @imports like stated above (although unfinished atm), 'UI' improvements. It's propose is neither to fork nor to publish it, discuss && cherrypick then accepted features would be the best ay to go imo :).

[RyanZim]

@michael-ciniawsky I'm currently not an official maintainer, just a user that watches the repo and responds to almost everything.

---

Have you done any work on the (uncompleted) incremental rebuild aspect?

[X-Tender]

No, I don't use output file, I use output dir.
The other example I mentioned was one input file and one output file, there
the watch process work. But I notice when I define and output dir to handle
multiple files. It is runs on the initial call and once on the first
change. But after that it doest do anything after a change.

Ryan Zimmerman notifications@github.com schrieb am Mo., 24. Okt. 2016,
17:18:

When I change it from dir to one css file it works without problems.

That's weird, postcss should error if you are using a single output file
with multiple inputs.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#44 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABWTW3dovclUpFe8M0r3EwTrtrjnqP_yks5q3MwqgaJpZM4KeIEP
.

[RyanZim]

@X-Tender Yeah, this bug isn't present if there is only one input file.

[watilde]

Hi, thanks for reporting this issue! I just tried to reproduce this issue, but I couldn't. Here is what I did: https://github.com/watilde/postcss-44

@RyanZim Thank you so much for your support as usual. Have you already got the point of the bug? It would be great if you could make a patch :) I'd love to ship it.

[RyanZim]

@watilde To reproduce, add content to a.css & b.css, run postcss-cli, and try changing the content in the files. The output files will not be reliably updated.

---

@michael-ciniawsky @watilde I'll try to put together a patch using msg.type === 'dependency'. Don't know if I'll get to it today.

[RyanZim]

Sorry so long in getting a patch together, might be next week; I'm currently quite busy trying to ship v1.0.0 over at https://github.com/jprichardson/node-fs-extra.

[michael-ciniawsky]

@RyanZim kk, nope nothing done in that regard, just came and farted the result.messagessolution 😛
@watilde Unrelated here but are you open for common config support postcss-load-config in postcss-cli?

[RyanZim]

This is blocked until postcss/postcss-import#233 is fixed.

[RyanZim]

postcss-import is up in the air: postcss/postcss#906.

[RyanZim]

postcss/postcss-import#233 is done, will try to get a patch together soon.

[RyanZim]

@watilde @ai @michael-ciniawsky Not sure exactly how we should do this. postcss-cli needs to use onImport or dependency messages, it can't use both.

postcss-import prior to v8.2.0 doesn't support dependency messages. If we stop watching onImport is that a breaking change?

[michael-ciniawsky]

Not sure exactly how we should do this. postcss-cli needs to use onImport or dependency messages, it can't use both.

@RyanZim I'm in favour of dependency message here, for that reason alone that it is a postcss core 'feature' available to every plugin in need of that feature, not plugin specific like onImport . Whenever possible imo plugin(s) should handle/use files/paths/deps in a common way which is plugin agnostic. => result.messages

postcss-import prior to v8.2.0 doesn't support dependency messages. If we stop watching onImport is that a breaking change?

Yep :) but when both are updated in parallel, new way works-out-of-box on both sides and it enhances the user experience, like it seems to be the case here, I would say party hard 🍾 😛

[RyanZim]

@michael-ciniawsky Good points.

Yep :) but when both are updated in parallel, new way works-out-of-box

Do you think this should be semver-major or not?

[michael-ciniawsky]

Do you think this should be semver-major or not?

@RyanZim Ff common config support (postcss.comfig.js) lands with this change in one blow, then definitely since config.json -> postcss.config.js. Standalone also I would say, but that is @watilde last call 😛 if

[RyanZim]

@watilde I am ready to make a PR for this. If this is to be released in v3, it should be based on #46. Could you possibly merge that PR into a branch named develop so I can submit a PR for that branch?

[RyanZim]

This will be fixed in v3.

[RyanZim]

Fixed on the develop branch, will be released in v3.