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

Remove need for node when generating source maps #39

Closed
harthur opened this issue Jun 23, 2014 · 4 comments · Fixed by #40
Closed

Remove need for node when generating source maps #39

harthur opened this issue Jun 23, 2014 · 4 comments · Fixed by #40

Comments

@harthur
Copy link

harthur commented Jun 23, 2014

I'm using this library in the browser by bundling it with browserify. Everything works fine except when I want to generate source maps. I'll get an error because fs.readFileSync isn't defined. The code path that uses readFile doesn't seem necessary to generate source maps and seems to only be used to apply any original source maps (I'm a bit unclear on that use case):

exports.applySourceMaps = function() {
  Object.keys(this.files).forEach(function(file) {
    var content = this.files[file];
    this.map.setSourceContent(file, content);

    var originalMap = sourceMapResolve.resolveSync(
      content, file, fs.readFileSync);
    if (originalMap) {
      var map = new SourceMapConsumer(originalMap.map);
      var relativeTo = originalMap.sourcesRelativeTo;
      this.map.applySourceMap(map, file, urix(path.dirname(relativeTo)));
    }
  }, this);
};

Right now I'm commenting that part out, but it would be nice to have it work out of the box. If there's any way to put the readFile part behind a conditional, that would be great.

@conradz
Copy link
Contributor

conradz commented Jun 23, 2014

Yeah, I think we should have an option to disable applying original source maps (this is to convert source maps of generated input files, such as Sass, to the output source map). Will work on this.

@ai
Copy link

ai commented Jun 24, 2014

Best settings is no settings :). Maybe if ( fs.readFileSync ) { … } check will be better? Like in PostCSS

@conradz
Copy link
Contributor

conradz commented Jun 24, 2014

The reason I added an option for this is so that (a) behavior does not unexpectedly change when running in browser and (b) this option may be useful for people who do not want this but are running it on Node.

@harthur
Copy link
Author

harthur commented Jun 26, 2014

As someone using it, I'm fine with specifying a setting, and to me that seems like it would be better, fwiw.

conradz added a commit that referenced this issue Jun 27, 2014
Add option to disable processing input source maps (fixes #39)
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 a pull request may close this issue.

3 participants