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

Add flag to disable inline sourcemap comment #3398

Closed
jfennell-techempower opened this issue Jun 4, 2019 · 11 comments
Closed

Add flag to disable inline sourcemap comment #3398

jfennell-techempower opened this issue Jun 4, 2019 · 11 comments

Comments

@jfennell-techempower
Copy link
Contributor

In addition to inline comments of the form:
/*# sourceMappingURL={your-file.css.map}*/,

The sourcemap V3 spec also defines the SourceMap HTTP header https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/SourceMap, which links the location of a file's accompanying sourcemap.

It would be great to have a compiler flag which disables the inline sourcemap comment, for the case where you wish to generate sourcemaps, but use the HTTP header instead of the inline comment to specify the location of your sourcemaps.

@jfennell-techempower
Copy link
Contributor Author

This seemed like a pretty trivial change, so I attempted a shot at it.

master...jfennell-techempower:add-flag-to-disable-sourcemap-url-annotation

I do not have time to write unit tests, but it seems to be working as intended. Feel free to fork my version, add tests as appropriate, and open a pr with the changes.

@stale
Copy link

stale bot commented Oct 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@matthew-dean
Copy link
Member

@jfennell-techempower

It would be great to have a compiler flag which disables the inline sourcemap comment, for the case where you wish to generate sourcemaps, but use the HTTP header instead of the inline comment to specify the location of your sourcemaps.

Is this your actual use case? You didn't really explain it other than a "nice-to-have". If it's just to have conformance with that spec, that's not a strong argument with this feature, as a comment adds little extra weight to a file.

@jfennell-techempower
Copy link
Contributor Author

At the time I was working on a project where we compiled our less server-side during development so we could have page load speeds similar to production, instead of needing to compile client-side on each page load and have a few seconds of waiting for the styles be compiled and applied to the DOM. We did not want sourcemaps to be served in production, however, for various reasons. Since the compiler always generated the comment, the browser would always attempt to load the sourcemaps in production, even though we weren't serving them, so the developer console and network inspector would be littered with errors. If this change were introduced, we could have disabled the sourcemap comment altogether in the compiler, and then added a header to our css asset responses in development with the location of the sourcemaps, so the browser would attempt to load them in development, but not in production.

@jfennell-techempower
Copy link
Contributor Author

It is worth mentioning as well that we had an incremental build process, in which we were re-using the css files compiled server-side during development when producing builds for staging and production. We previously had a build step that took well over 2 minutes to compile all of the less before it was converted to an incremental compilation (compared to an overall build time of 5-10 seconds when building incrementally). It was undesirable from a performance standpoint to have two separate build steps - the incremental build step in dev where we only compiled files that changed, and a full-compilation in production where we excluded the sourcemap flag, to get rid of the sourcemap comments in the css files. Not to mention, we liked the architectural simplicity and cross-environment parity of using the same compilation step in development and production. That is why the answer wasn't as simple as "Just use a separate invocation of the compiler without the sourcemap flags when building for production".

@matthew-dean
Copy link
Member

matthew-dean commented Oct 17, 2019

@jfennell-techempower

That is why the answer wasn't as simple as "Just use a separate invocation of the compiler without the sourcemap flags when building for production".

Hmm, ok, interesting. That's what I was about to ask. So, essentially, the feature request is to generate the source map, without that flag generating the source map comment in the .css.

It still feels like an edge case that could be managed by some kind of text post-processing in your build setup? That is, I wouldn't imagine this would be a typical use case.

@jfennell-techempower
Copy link
Contributor Author

jfennell-techempower commented Oct 18, 2019

It still feels like an edge case that could be managed by some kind of text post-processing in your build setup? That is, I wouldn't imagine this would be a typical use case.

Perhaps not. I personally would not want to introduce another processing step in a compilation step, as I feel that the result of a compilation shouldn't need additional processing, and that it is simple and reasonable enough to just not generate the comment at all. But I respect if the changes I pushed in the branch above are not sufficient, and this isn't something that enough people need to warrant dedicating additional time to it. I just figured I'd try, since the specification does allow the option of using an HTTP header instead of an inline comment, and the compiler does not grant developers the flexibility to make that decision currently.

Thanks for considering!

@susnet
Copy link

susnet commented May 19, 2020

This is for sure a use-case when using JSF (Java Server Faces) resources because then you need to add .xhtml at the end in order for the browser to find the generated map file. To be able to use the HTTP header to solve this you need to disable the inline sourcemap comment in order for the HTTP header to take precedence. If you don't fix this then you cannot use this: http://showcase.omnifaces.org/resourcehandlers/SourceMapResourceHandler that would have solved this easily.

@matthew-dean
Copy link
Member

@susnet Changed it to "up-for-grabs"

@hirosato
Copy link
Contributor

Let me try this.
I've cherry-picked a commit from #3398 (comment)
I'll start checking how tests are written.

@hirosato
Copy link
Contributor

I've submitted a PR for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants