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

Can't get hot module replacement to work with sass-loader version newer than 0.2.0 #47

Closed
justin808 opened this issue Jan 31, 2015 · 17 comments

Comments

@justin808
Copy link
Contributor

I'm trying to update my rails-webpack-reactjs tutorial project to the latest dependencies.

https://github.com/justin808/react-webpack-rails-tutorial/tree/update-to-latest

The only dependency that does not work is sass-loader. Any version greater than 0.2.0 gives the following error:

ERROR in ../~/css-loader!../~/sass-loader?outputStyle=expanded&imagePath=/assets/images&includePaths[]=/Users/justin/j/react/react-webpack-rails-tutorial/webpack/assets/stylesheets!./assets/stylesheets/test-sass-stylesheet.scss
TypeError: Object function (from) {
            return !!~this.indexOf(from);
        } has no method 'replace'

Thus, I'm stuck at 0.2.0. Any ideas on what I can do further pinpoint what change broke my app?

Has anybody else gotten the hot-reload server to work with a version greater than 0.2.0?

Thanks.

@jhnns
Copy link
Member

jhnns commented Jan 31, 2015

Mhmm I'm not quite sure ... could be an issue with sass-graph. Could you remove the calls of markDependencies() in your local installation of the sass-loader and check, if the problem still exists?

@justin808
Copy link
Contributor Author

That does solve the error. What do you recommend next?

@justin808
Copy link
Contributor Author

@jhnns I'm debugging your latest with the latest from sass-graph (1.0.1). Any reason you're locked in at an older version of sass-graph?

@justin808
Copy link
Contributor Author

@jhnns I've tracked down the problem to sass-graph using for(var a in someArray) syntax and one of my dependencies must have added a method to array. Thus, crash, just as explained here: http://stackoverflow.com/a/3010848/1009332

https://github.com/xzyfer/sass-graph/blob/master/sass-graph.js#L54

  for (var i in imports) {
    [this.dir, cwd].forEach(function (path) {
      if (path && this.loadPaths.indexOf(path) === -1) {
        this.loadPaths.push(path);
      }
    }.bind(this));
    var resolved = resolveSassPath(imports[i], _.uniq(this.loadPaths));
    if (!resolved) return false;

    // recurse into dependencies if not already enumerated
    if(!_.contains(entry.imports, resolved)) {
      entry.imports.push(resolved);
      this.addFile(fs.realpathSync(resolved), filepath);
    }
  }

@akiran
Copy link
Contributor

akiran commented Feb 1, 2015

@justin808,
We submitted a pull request to sass-graph to fix some of the issues with sass-loader.
Since npm release was not made at that time, we tagged it to git commit.

Now @xzyfer is maintaining sass-graph and an npm release was made recently.
We should test it with sass-loader and migrate to latest version of sass-graph.

@justin808
Copy link
Contributor Author

@akiran is there a fork/branch I should pull? I'm going to check that just changing the array iteration fixes my issue.

@akiran
Copy link
Contributor

akiran commented Feb 1, 2015

Now sass-graph is maintained at
https://github.com/xzyfer/sass-graph

@xzyfer
Copy link

xzyfer commented Feb 1, 2015

Hey all, feel free to open issues and PR in sass-graph. I'm actively maintaining it.

There's a lot a clean up work to be done, which is coming.

@justin808
Copy link
Contributor Author

This was blocking me:

xzyfer/sass-graph#7

Any ETA on when we might be able to see this fix and an update to sass-loader? I'd rather use the official versions than my own fork. Thanks! And if you're interested, please check out my sample project on using rails with webpack and react: https://github.com/justin808/react-webpack-rails-tutorial/tree/update-to-latest.

justin808 added a commit to justin808/sass-loader that referenced this issue Feb 1, 2015
* Addresses blocking issue where any additions to Array.prototype cause
  a crash. See webpack-contrib#47
@xzyfer
Copy link

xzyfer commented Feb 1, 2015

I've had a preliminary look at xzyfer/sass-graph#7. I have some feels about it.

One way or another expect a new npm release with this issue patched within 24hrs.

@xzyfer
Copy link

xzyfer commented Feb 2, 2015

I've published sass-graph@1.0.2 that should address the issue raised by @justin808 .

@justin808
Copy link
Contributor Author

I posted a fix to sass-loader to reference my own fork of sass-graph.

However, sass-graph just fixed my reported issue.

commits_ _xzyfer_sass-graph

Please comment in this issue when this version sass-loader is updated to this version of sass-graph.

@akiran
Copy link
Contributor

akiran commented Feb 2, 2015

Did latest version of sass-graph fixed your issues ?

Then we can update sass-loader and make an npm release.

@xzyfer
Copy link

xzyfer commented Feb 2, 2015

I've just published a sass-graph@1.0.3 which passes the test case added by @justin808 .

@justin808
Copy link
Contributor Author

@akiran Can we update sass-loader? I just submitted a PR, albeit a very simple one, to update the version dependency.

@jhnns
Copy link
Member

jhnns commented Feb 11, 2015

Fixed?

@jhnns
Copy link
Member

jhnns commented Mar 22, 2015

We don't depend on sass-graph anymore.

@jhnns jhnns closed this as completed Mar 22, 2015
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

No branches or pull requests

4 participants