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

fix(index): don't set to value (options) #339

Merged
merged 2 commits into from
Feb 26, 2018
Merged

fix(index): don't set to value (options) #339

merged 2 commits into from
Feb 26, 2018

Conversation

camshaft
Copy link
Contributor

@camshaft camshaft commented Feb 25, 2018

ℹ️ Describe the big picture of your changes here to communicate to the maintainers
why we should accept this pull request. If it fixes a bug or resolves a feature
request, be sure to link to that issue.

When passing to it truncates the output source map file path to just the path base name. By removing this value the paths are now correct.

The implementation of this logic is found here

Type


ℹ️ What types of changes does your code introduce?

  • Fix

SemVer


ℹ️ What changes to the current semver range does your PR introduce?

  • Bug (:label: Patch)

Issues


ℹ️ What issue (if any) are closed by your PR?

  • None

Checklist


ℹ️ You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. This is a reminder of what we are going to look for before merging your code.

  • Lint and unit tests pass with my changes
  • I have added tests that prove my fix is effective/works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes are merged and published in downstream modules

When passing `to` it truncates the output source map file path to just the path base name. By removing this value the paths are now correct.

The implementation of this logic is found [here](https://github.com/postcss/postcss/blob/master/lib/map-generator.es6#L179-L195)
@coveralls
Copy link

coveralls commented Feb 25, 2018

Coverage Status

Coverage remained the same at 87.975% when pulling aa52605 on camshaft:patch-1 into 0a643de on postcss:master.

@michael-ciniawsky michael-ciniawsky changed the title Remove to options value fix(index): don't set to value for source maps Feb 25, 2018
@michael-ciniawsky michael-ciniawsky changed the title fix(index): don't set to value for source maps fix(index): don't set to value (options) Feb 25, 2018
Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

@camshaft Thx, Could you show a screenshot of the devTool panel for reference, if this definitely fixes the issue ?

@camshaft
Copy link
Contributor Author

Without this fix:

2018-02-25-172627_435x528_scrot

With this fix:

2018-02-25-172511_362x390_scrot

@michael-ciniawsky
Copy link
Member

michael-ciniawsky commented Feb 26, 2018

Released in v2.1.1 🎉. Thx

This was referenced Mar 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants