akanix42 / meteor-react-toolbox-example

MIT License
18 stars 1 forks source link

SCSS changes require meteor reset to take #1

Closed alexkramer closed 8 years ago

alexkramer commented 8 years ago

I am finding that if I update the toolbox-theme.scss variables, the changes will be served the initial time however all subsequent changes will only take after first performing a meteor reset. I figure I am either missing a step or this is related to some meteor build issue (I checked their backlog but couldn't find anything that was clearly related).

Note: I am using OSX so I had to manually install your css-modules package since I was unable to fetch the most recent version from Atmosphere.

akanix42 commented 8 years ago

The toolbox-theme file is a little special because of it's loaded; currently, changes to it should refresh whenever you restart your Meteor app. I'll fix this so they're reloaded every time the build process every runs.

akanix42 commented 8 years ago

The react toolbox example is still on css-modules@1.0.0; if you update to 1.0.1, you'll be able to pull the package from atmosphere instead of manually installing it.

alexkramer commented 8 years ago

Awesome, I will update my dependencies and remove the local install Thanks! I did walk through all of the code today and reviewed the Meteor build process and plugin classes. I was able to determine that Meteor was detecting the file change, was calling your plugin for processing, and that the processFilesForTarget method did have cache misses in this instance (which is what I would have expected to happen). Ultimately, however, no changes were pushed to the merged-stylesheets.css file. Even a clean restart of the application results in no changes being reflected. I found only two methods for having the changes propagate:

  1. Meteor reset, followed by Meteor run
  2. Making some random change to your local plugin (like adding a logging statement) which would trigger a complete rebuild of the plugin and then the changes made to the variables in the toolbox-theme file would take and be reflected in the client
akanix42 commented 8 years ago

Ah, thank you very much for that detailed response, that really sheds light on the issue: Changes to the "globalVariables" setting need to rebuild the other cached files.

akanix42 commented 8 years ago

Caching is disabled in nathantreid:css-modules@1.0.3 as a temporary workaround until I get the cache invalidation implemented.

alexkramer commented 8 years ago

Sounds good. I did try and give it a go. I found that the app will not render. Its throwing errors in the console like:

Uncaught Error: Cannot find module './style'

Have you seen this before? Does the master branch of this project work for you using Meteor 1.3.1 and version 1.0.3 of the css-modules? I feel like if anything this may be an issue with 1.3.1. I tried trolling their bug list but I only had a few minutes to look at it tonight.

akanix42 commented 8 years ago

Yes, this is caused by a CSS Modules bug fix I made yesterday; which necessitates a change to the outputJsFilePath when loading React Toolbox. I updated the project & documentation but hadn't merged it onto the master yet (it's there now). Here's the correct syntax:

"cssModules": {
    "outputJsFilePath": {
      "node_modules/react-toolbox/.*": "{dirname}/{basename}.js"
    }
}

You no longer need to change the default handling because if you leave off the file extension when importing e.g. import style from './style';, Meteor automatically checks all handled file extensions. However, this doesn't work for node_modules, because Meteor currently only allows importing .js and .json files from there, so we have to trick Meteor into seeing the React Toolbox style.scss files as style.js.

alexkramer commented 8 years ago

Beautiful, thanks! I was able to get it back up and running with that tweak and changes to the global variable files were successfully updating the view without requiring a reset or rebuild.

akanix42 commented 8 years ago

I'm going to close this since it's working now and I have a separate issue to track the cache bug.