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 support for source maps #123

Open
sitegui opened this issue Dec 11, 2015 · 12 comments
Open

Add support for source maps #123

sitegui opened this issue Dec 11, 2015 · 12 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@sitegui
Copy link

sitegui commented Dec 11, 2015

Hi all,

In PR #56, it was discussed one can modify the EJS source before handing it to ejs.compile().
It works, but has the downside of making the extended error messages (with EJS source and line numbers) hard to read, since all source will probably be presented as one line.

Another solution is to minify the render()'s output, but this may be too expensive. In my benchmarks, using html-minifier, it takes more than 100ms (for my use case, that's too much).

But I think offering HTML minification as part of EJS is not the way to go either, since it would bloat this wonderful module too much and would not solve the underlying problem with pre-processed source.

What if EJS could accept a source map as part of the compilation and use it to report the right line number and original source? This way, HTML minification could be part of another module, separating concerns.

I'd like to hear you position on this issue: do you think this module should accommodate this use case?

Best regards,
Guilherme

@mde
Copy link
Owner

mde commented Dec 11, 2015

This sounds great, if it can be implemented as an optional module. Then this could be added to the lib without creating a bunch of bloat. All that would need to be added would be the hooks.

@RyanZim RyanZim added the enhancement New feature or request label Mar 24, 2016
@RyanZim
Copy link
Collaborator

RyanZim commented Apr 16, 2016

@sitegui, are you interested in working on this? Otherwise, I'll add a help wanted label.

@sitegui
Copy link
Author

sitegui commented Apr 16, 2016

Hi @RyanZim,

I'm not interested in taking this any longer, you can add the label ;)

For my use case, I've implemented a version of EJS that understands HTML, so that it can run the minification logic on compile time. It also allowed me to support a bunch of other HTML-related stuff (like linting and custom elements).

@RyanZim RyanZim added the help wanted Extra attention is needed label Apr 16, 2016
@RyanZim RyanZim changed the title Minify HTML in EJS (or add support for source map?) Add support for source maps Apr 27, 2016
@madcampos
Copy link

Hi folks,
I was reviewing info about source maps, mostly the spec and the html5 rocks article and was thinking: what will be translated to the source map?
In JS/CSS the classes, functions, objects and other constructs are translated from the generating language in a 1 to 1 mapping. Will it be the same here? What parts of EJS will be translated to HTML, and to what in html? Tags?
I'm curious so that I can help with this implementation.

@mde
Copy link
Owner

mde commented Oct 25, 2016

This is a really interesting question -- internally EJS maintains its own mapping of template source-code so it can provide correct error lines for thrown errors. I'm not even sure what minifying means here, because we could be talking about minifying the template source-code, or minifying the generated JavaScript source code.

I guess this would actually mean creating a traditional sourcemaps file for the generated JS source-code that would plug into the existing tracked line numbers in the original template source. This is a technically very interesting and challenging project.

@RyanZim
Copy link
Collaborator

RyanZim commented Oct 25, 2016

Before we go too far on this, a few questions:

  • Source maps work for JS & CSS. Is there any browser capable of handling HTML source maps?
  • JS source maps are linked with //# sourceMappingURL=<url>, how would HTML maps be linked? I can't find anything in the spec other than the HTTP header (https://sourcemaps.info/spec.html#h.lmz475t4mvbx).
  • I know there was talk of HTML source maps in the past, but are there any template engines or HTML minifiers that actually produce or handle source maps? (@TimothyGu?) If not, there is little point in EJS implementing this.

@TimothyGu
Copy link
Collaborator

I'm not sure about EJS/HTML to HTML sourcemap, but I do think EJS to compiled JS sourcemap could be helpful. We would be able to delete the rethrow functions, in favor of more standardized facilities like https://github.com/evanw/node-source-map-support.

@RyanZim
Copy link
Collaborator

RyanZim commented Oct 26, 2016

@TimothyGu True; this wouldn't solve the original issue of handling html minification and keeping correct line numbers.

Another solution is to minify the render()'s output, but this may be too expensive. In my benchmarks, using html-minifier, it takes more than 100ms (for my use case, that's too much).

This begs the question: if we added sourcemaps, minified the the EJS templates, etc., would it be any faster?

@sitegui
Copy link
Author

sitegui commented Oct 26, 2016

My original issue was about keeping the correct mapping from EJS run-time exceptions to the original code, after it has run through a pre-processor.

My idea back there was to create a HTML minifier pre-processor so that, for example, the following EJS code:

<span class="user">
    <%= user.name %>
</span>

would be compiled to <span class=user><%= user.name %></span>.
So, say user is null, on rendering it would throw an exception and it should print the original three-line code and point to the error in the second line.

In this model, what is minified is the input to compile(), so the HTML minification logic is applied once, on startup. This answers:

This begs the question: if we added sourcemaps, minified the the EJS templates, etc., would it be any faster?

It would, if the template is compiled once and ran multiple times.

The original idea had nothing to do with outputing a source map, but instead with accepting one to map errors.

In the end, I've written ejs-html to handle HTML minification directly.

@RyanZim
Copy link
Collaborator

RyanZim commented Oct 26, 2016

Thanks for chiming in @sitegui

As things stand now, I would be in favor of closing this.

  • @sitegui has found a solution that works for him.
  • Someone would need to write the EJS minifier; unless we have a volunteer for that, there is no point discussing this.

@TimothyGu If you feel that line-number error mapping should/could be rewritten, that should be a separate issue. "If it ain't broke, don't fix it!" IMO, though.

If someone has something else to add to this discussion, please do so, otherwise, I will probably close in few days.

@TimothyGu
Copy link
Collaborator

No I definitely agree that it doesn't need to be rewritten, but if we have sourcemap support bridging with other sourcemap-aware tooling like Webpack could be easier, in addition to better rethrow.

@v4dkou
Copy link

v4dkou commented Dec 22, 2020

From what I see, to implement source maps, we need to pass this.currentLine to the compiled __append in the scanLine method. Then if we extend this __append method to store an association between the source line number and output lines range, we'll get something like a source map.

https://github.com/mde/ejs/blob/main/lib/ejs.js#L885
https://github.com/mde/ejs/blob/main/lib/ejs.js#L827

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants