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

Fixed Source Map generation when multiple input/output paths are used. #28

Closed
wants to merge 2 commits into from
Closed

Fixed Source Map generation when multiple input/output paths are used. #28

wants to merge 2 commits into from

Conversation

mkempe
Copy link
Contributor

@mkempe mkempe commented Oct 18, 2015

Hey there,

first of all, many thanks for the plugin.

This pull request fixes the generation of Source Maps when multiple input/output paths are used, for example in an Ember CLI app:

var app = new EmberApp(defaults, {
  outputPaths: {
    app: {
      html: 'index.html',
      css: {
        'foo': '/assets/foo.css',
        'bar': '/assets/bar.css'
      },
      js: '/assets/app.js'
    },

    // ...
  },

  // ...
};

Without the pull request, for /assets/bar.css a Source Map with the name /assets/foo.css.map is created instead of /assets/bar.css.map.

This is probably also the reason for gpoitch/ember-cli-less#32.

Maik

@knownasilya
Copy link
Contributor

Looking forward to this merging.

options.sourceMap = {};
}

options.sourceMap.sourceMapURL = this.outputFile + '.map';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Previously you were able to pass a sourceMapURL as part of the options.sourceMap hash (line 29 in original file). So I'm slightly confused or misunderstanding the issue. This PR removes the behavior.

@jasonmit
Copy link
Collaborator

Thanks for the PR. Left a question before I can merge, because I think I'm not fully understanding if this is a broccoli-less-single limitation or ember-cli-less. Since it does respect the sourceMapURL option.

Also, please refrain from cosmetic changes. I left it the way the original author intended it, and makes reviews more difficult.

@mkempe
Copy link
Contributor Author

mkempe commented Oct 21, 2015

Hey Jason,

many thanks for looking over the PR and your feedback. You are right, with the points you mentioned.
I just looked over the code and identified the "real" problem. The problem is the options handling in line 23: https://github.com/gabrielgrant/broccoli-less-single/blob/master/index.js#L23.

I will close this PR and open another which fixes and explains the problem.

PS: Sorry for the cosmetic noise.

@mkempe mkempe closed this Oct 21, 2015
@mkempe mkempe deleted the additional_output_paths branch October 21, 2015 09:57
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 this pull request may close these issues.

None yet

3 participants