Skip to content

fix(tasks): adjust styles watcher implementation#141

Open
Hugoer wants to merge 1 commit intoNetcentric:mainfrom
Hugoer:Update-chokidar-usage-to-support-v4
Open

fix(tasks): adjust styles watcher implementation#141
Hugoer wants to merge 1 commit intoNetcentric:mainfrom
Hugoer:Update-chokidar-usage-to-support-v4

Conversation

@Hugoer
Copy link
Copy Markdown
Member

@Hugoer Hugoer commented Mar 11, 2026

This PR fixes an issue with SCSS file watchers caused by Chokidar version > 3. In versions 4+, Chokidar no longer supports glob patterns directly.

Description

Downgraded chokidar from v5 to v4 to maintain NC FE Build compatibility (node version).

Use fast-glob to have the observer pre-filter the SCSS files and listen only to the specific list of files., instead of monitoring the entire ${config.general.sourcesPath} folder. This ensures reliable detection of SCSS changes and improves watcher performance.

Related Issue

Fixes #138

Types of changes

  • 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.
  • I have read the CONTRIBUTING document.
  • All new and existing tests passed.

@easingthemes
Copy link
Copy Markdown
Member

Issues Found

  1. Missing glob import (Blocker)
    The diff adds await glob(pattern, ...) but there’s no visible require('fast-glob') or const { glob } = require('fast-glob') import at the top of tasks/styles.js. The existing file only imports path, log, generateEntries, and renderStyles. This will throw a ReferenceError: glob is not defined at runtime.
    Fix needed: Add const { glob } = require('fast-glob'); (or const glob = require('fast-glob');) at the top of the file.
  2. async Promise constructor anti-pattern (Minor)
    The promise callback is changed to new Promise(async (resolve) => { ... }). This is a well-known anti-pattern because:
    ∙ Exceptions thrown inside an async executor are not caught by the Promise — they become unhandled rejections instead of rejecting the returned promise
    ∙ The existing try/catch mitigates this partially, but any error from await glob(...) that escapes the catch block will be silently swallowed
    Suggested improvement: Either:
    ∙ Move the async logic before the Promise constructor, or
    ∙ Use async/await throughout and remove the new Promise wrapper entirely (preferred — the function could just be module.exports = async (config) => { ... })
  3. Watcher won’t detect new SCSS files (Design consideration)
    Since fast-glob resolves files once at startup, newly created SCSS files won’t be watched. The old glob-based approach (when it worked) would have picked up new files. This may be acceptable given the EMFILE constraints mentioned in Update chokidar usage to support v4 (glob patterns removal) #138, but it’s worth documenting as a known limitation.
  4. Downgrade vs. upgrade reasoning (Minor)
    The PR description says “downgraded chokidar from v5 to v4” but the title of Update chokidar usage to support v4 (glob patterns removal) #138 says “support v4.” The commit message says “update chokidar to version 4.0.3.” Since v5 existed before, this is technically a downgrade. Worth clarifying in the PR description whether v5 introduced other issues (the EMFILE problems mentioned in the issue?).
    Verdict
    Changes requested — The missing glob import (Rename template vars with fe-build one #1) is a runtime blocker that must be fixed before merge. The async Promise anti-pattern (Add package code  #2) is worth addressing. Items Order imports alphabetically #3 and stylelint.config.js #4 are informational.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update chokidar usage to support v4 (glob patterns removal)

2 participants