-
Notifications
You must be signed in to change notification settings - Fork 465
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
API: source-map related options enabling criteria #885
Comments
I can confirm that specifying |
IMO this would mostly just mean to implicitly enable |
I agree with @am11, I believe I may have originally instigated this report. IMHO it's poor UX to have to enable Do we even need |
@xzyfer, yes that option is required: From libsass' API angle, If
I prefer the latter one because it will not confuse the user and if someone comes to our shop with crazy use-case, we can easily pitch: what you set is what you get. 😄 |
Also, neither libsass nor node-sass do anything with
|
Can I get clarification on my earlier question please: is it expected that specifying |
I have made |
With growing source-map related options, there has been some confusion among node-sass users.
node-sass takes the user options and send to LibSass as is (at the risk of default values
undefined
type casted tonull
).I believe there is a need of a well-defined policy. to make the "require options to set" criteria for each case. I propose to make this kind of policy as relaxed as possible, i.e. let users set minimum options to enable the intended feature. To devise such a policy, some changes might be required.
Copying from sass/node-sass#622 (comment), here is an extract of what we have found:
source_map_contents == true
should emit source-map without needing the presence ofsource_map
.source_map_contents == true
should emit source-map without needing the presence ofoutput_path
.source_map_embed == true
should emit source-map without needing the presence ofsource_map
.source_map_embed == true
should emit blank string onsass_context_get_source_map_string(ctx)
. (NEW based on Added support for sourcesContent and update to node-sass v2.0.1 joliss/broccoli-sass#43 (comment)).source_map_embed == true
should emit source-map without needing the presence ofinput_path
, whendata
option is set.source_map_embed == true && source_map_contents == true
should emit source-map without needing the presence ofoutput_path
. (hint: emitfile: ''
in JSON)source_map_embed == true && source_map_contents == true
should emit source-map without needing the presence ofinput_path
whendata
option is set.(note: there might be more considerations here that I am missing ..)
Once agreed upon and LibSass API defines the policy to set which options to enable what, we will use the same in node-sass README (or hopefully wiki) for node-sass API implementers.
The text was updated successfully, but these errors were encountered: