Skip to content
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

Build: Add CSS files in /public/sections to the watched hot-reload files list #19228

Merged
merged 1 commit into from
Nov 6, 2017

Conversation

ryelle
Copy link
Member

@ryelle ryelle commented Oct 27, 2017

After #18823 was merged, I noticed that hot-reloading CSS wasn't working in the Store extension anymore. I traced this to a list of watched CSS files, publicCssFiles, in /server/bundler/css-hot-reload.js. The very simple solution I've come to here is just adding the files in /public/sections to this list as well.

There are a few things I noticed, but I don't think they're necessarily problems:

  • It always reloads style-debug.css, even though I'm pretty sure nothing's changed there
  • It reloads section files regardless of whether you're on that section— if I make a change in the store extension CSS, it reloads onto any calypso.localhost, even if I'm not on a store page.

This still doesn't watch the files in public/sections-rtl, and at this point I'd suggest either something like recursive-readdir-sync, or looping over an array whitelist of PUBLIC_DIRs.

To test

  • Start calypso: npm start
  • Load up a store page, ex the dashboard: /store/:site (where site is an atomic site)
  • Make a change in store CSS (extensions/woocommerce/style.scss)
  • Open the console and watch for Building CSS…, Reloading CSS… <…woocommerce.css
  • See your changes 🎉

@ryelle ryelle added Build [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Type] Bug When a feature is broken and / or not performing as intended labels Oct 27, 2017
@ryelle ryelle self-assigned this Oct 27, 2017
@matticbot
Copy link
Contributor

Copy link
Contributor

@allendav allendav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works very nicely. :shipit:

Copy link
Contributor

@coderkevin coderkevin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, tests good! :shipit:

@ryelle ryelle removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 2, 2017
@ryelle ryelle force-pushed the fix/sections-hot-reloading branch from cd6a751 to 9292c07 Compare November 2, 2017 21:36
@ryelle ryelle added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 2, 2017
@ryelle ryelle merged commit 5126c3f into master Nov 6, 2017
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 6, 2017
@ryelle ryelle deleted the fix/sections-hot-reloading branch November 13, 2017 15:27
rclations pushed a commit to rclations/wp-calypso that referenced this pull request Nov 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants