bhauman / figwheel-main

Figwheel Main provides tooling for developing ClojureScript applications
https://figwheel.org
Eclipse Public License 1.0
640 stars 93 forks source link

Figwheel doesn't understand CSS @imports #129

Closed KingMob closed 5 years ago

KingMob commented 5 years ago

If we have a parent.css in the HTML that does @import "child.css";, updates to child.css will not be reloaded by Figwheel.

The current behavior is reload-css-file calls get-correct-link which calls current-links, which only looks at <link> tags already in the document.

FWIW, I know goog.cssom.getAllCssStyleSheets() includes imports correctly.

I can tackle this if you want to include it, but it might take a few weeks with the holidays before I have some time.

bhauman commented 5 years ago

Oh this is interesting, but not trivial as you have to trace dependencies, correct?

But bringing more sophistication to the process would be interesting.

KingMob commented 5 years ago

After poring over goog.cssom, I don't think it should be too bad to implement, since it traces the deps for you. When calling goog.cssom.getAllCssStyleSheets(), it recurses on @import rules, and produces a completely ordered list. (Since @imports can only be at the start of a CSS file, there's never any worries about splicing into the middle of a sheet's rules.) If we need to go back up a deps tree, there's the .parentStyleSheet property.

Options

I see several possibilities. Let me know what you think.

  1. Reload the top-level file. We use the complete, flattened list of stylesheets for lookups only. I.e., when a file in the watched CSS dir changes, locate that file in the complete list, read .parentStyleSheet repeatedly to find its top-level parent CSS file, and then reload at the top-level.
    • Pros: Relatively few code changes.
    • Cons: Will reload files that haven't changed, potentially all of them if you have just one top-level CSS file that @imports the rest.
  2. Reload the child file when it changes. Like above, we obtain a complete list of stylesheets, and when a watched file changes, we look up that file in the list, and locate its immediate parent stylesheet. In the parent, we find the associated CSSImportRule, remove it, and re-add it to the parent, forcing it to reload just that child.
    • Pros: More targeted, should reload fewer files.
    • Cons: Probably a bit more work than option 1. New code path for import-based reloads. Can still reload un-changed children of children.
  3. Change individual rules. When a watched CSS file changes (parent or child), look up the URL, fetch it via XHR, diff the old and new CSS rules and insert/update/remove only the changed rules, leaving the rest of the stylesheet untouched.
    • Pros: Absolute minimum changeset. Can avoid re-importing child files when a parent changes, if the changes are after the imports.
    • Cons: Probably a lot of work for little benefit. Something like LCS diffing will be an N^2 algorithm, which, for large CSS files, could potentially dwarf the cost of just re-inserting everything. Adds a networking requirement. Might be some CORS issues between styles and XHRs?

I personally think option 2 is a good trade-off, followed by option 1. Option 3 is probably overkill, but if the CSS was huge or the DOM mega-slow, it might be worth considering.

KingMob commented 5 years ago

@bhauman, have you had a chance to think about it?

bhauman commented 5 years ago

I'm leaning towards option 1 because of its simplicity. Reloading extra CSS is never really a problem.

KingMob commented 5 years ago

OK, I'll work on option 1 and get back to you when I get a chance.