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: respect map.annotation string #307

Merged
merged 4 commits into from Jan 9, 2020

Conversation

keithamus
Copy link
Contributor

As per the PostCSS docs on sourcemaps the opts.map.annotation option can be set to a string, to write a manually named sourcemap file:

  • annotation boolean or string: indicates that PostCSS should add annotation
    comments to the CSS. By default, PostCSS will always add a comment with a path
    to the source map. PostCSS will not add annotations to CSS files that
    do not contain any comments.

    By default, PostCSS presumes that you want to save the source map as
    opts.to + '.map' and will use this path in the annotation comment.
    A different path can be set by providing a string value for annotation.

    If you have set inline: true, annotation cannot be disabled.

This PR fixes PostCSS-cli to acknowledge this option, and refactors getMapfile to return opts.map.annotation if the option is set to a string.

@keithamus keithamus force-pushed the fix-respect-map-annotation-string branch from 618cb7d to d7ac29d Compare January 8, 2020 18:24
Copy link
Collaborator

@RyanZim RyanZim left a comment

Choose a reason for hiding this comment

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

Code looks good to me.

There's a unit test for lib/mapFile.js here that needs fixing since the function signature changed:

t.is(getMapfile(p.input), p.want)

Also, an integration test using map.annotation would be great, although not required.

@keithamus
Copy link
Contributor Author

@RyanZim I've fixed up the unit tests to take the correct objects. I've also added an extra case to show what happens when annotation is provided.

Happy to add extra integration tests if you feel them necessary. Could you please point to where I might add them?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 76.316% when pulling 3b09732 on keithamus:fix-respect-map-annotation-string into 745ad2c on postcss:master.

@coveralls
Copy link

coveralls commented Jan 9, 2020

Coverage Status

Coverage increased (+0.5%) to 76.316% when pulling 9a5e383 on keithamus:fix-respect-map-annotation-string into 745ad2c on postcss:master.

Copy link
Collaborator

@RyanZim RyanZim left a comment

Choose a reason for hiding this comment

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

LGTM

@RyanZim RyanZim merged commit 568528d into postcss:master Jan 9, 2020
@RyanZim
Copy link
Collaborator

RyanZim commented Jan 9, 2020

Released in 7.1.0 🎉

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.

None yet

3 participants