ejs icon indicating copy to clipboard operation
ejs copied to clipboard

Add support for source maps

Open sitegui opened this issue 9 years ago • 12 comments

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

sitegui avatar Dec 11 '15 00:12 sitegui

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.

mde avatar Dec 11 '15 07:12 mde

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

RyanZim avatar Apr 16 '16 18:04 RyanZim

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).

sitegui avatar Apr 16 '16 19:04 sitegui

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.

madcampos avatar Oct 25 '16 05:10 madcampos

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.

mde avatar Oct 25 '16 05:10 mde

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.

RyanZim avatar Oct 25 '16 11:10 RyanZim

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.

TimothyGu avatar Oct 26 '16 18:10 TimothyGu

@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?

RyanZim avatar Oct 26 '16 18:10 RyanZim

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.

sitegui avatar Oct 26 '16 18:10 sitegui

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.

RyanZim avatar Oct 26 '16 19:10 RyanZim

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.

TimothyGu avatar Oct 30 '16 02:10 TimothyGu

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

v4dkou avatar Dec 22 '20 03:12 v4dkou