#3529 Suggestion: revert back to lodash templates, albeit dropping use of the 'with' statement

[Anonymous]

Originally created by: jayaddison

Recently for release v7.4.0 v7.4.1 of the workbox packages, the JavaScript templating engine used in the workbox-build package was changed from lodash templating to eta templating.

Some of the benefits of eta are that it has a reduced dependency-set compared to lodash, and that the migration resolved an open bug in the v4.18.0 release of lodash.

I am relatively conservative about changing dependencies, and would like to suggest that we consider switching back to lodash templates. The reasons I have in mind are:

• Although eta itself has a smaller dependency-set, lodash is already, and remains, a dependency (now transitive) of workbox-build -- so we have not reduced the installed dependency set; arguably we may have reduced (or at least changed) the runtime code paths, though.
• The error raised during use of v4.18.0 of lodash was due to a faulty release, and that has been resolved with v4.18.1 -- it's not ideal that a release fault could occur, and I don't precisely understand what went wrong, but unless there is a significant known problem, I don't find that it's compelling enough to entirely replace the templating engine.
• The community size for lodash and its templating engine are large -- this aids the ability to find, report and resolve problems.
• Although there have been some security issues with lodash templating, those have been resolved and the maintainers seem responsive.
• One of the open issues in the eta bugtracker makes me think that the way that it parses template files may be fragile:
  • https://github.com/bgub/eta/issues/337
  • NB: there was an open pull request to address this problem (thanks @rtritto for finding that PR), but it seems to have disappeared recently.

However, the migration to eta did cause me to learn about the JavaScript with statement that is used by default in both the lodash templating and eta templating engines. It is deprecated: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/with

I would like to suggest that workbox consider changing back to use the lodash templating engine for workbox-build, albeit enabling the variable option at the same time to remove the dependency on the deprecated JavaScript with statement.

cc @rtritto @swissspidy in case you have any thoughts from involvement in this process.

Thanks!

Edit: fixup: this migration took place for v7.4.1 of workbox, not v7.4.0 as originally described here

[Anonymous]

Originally posted by: rtritto

Although there have been some security issues with lodash templating, those have been resolved and the maintainers seem responsive.

lodash.template was officially deprecated This package is deprecated. Use https://socket.dev/npm/package/eta instead. (see lodash.template package (www.npmjs.com)).
Probably maintainers of lodash stop updates of template inside lodash and it isn't removed due to backport or to avoid a breaking change.
eta is a dedicated package for templating that's constantly updated. We get more advantages on using eta instaead of lodash.template.

One of the open issues in the eta bugtracker makes me think that the way that it parses template files may be fragile:

https://github.com/bgub/eta/issues/337
NB: there was an open pull request to address this problem (thanks @rtritto for finding that PR), but it seems to have disappeared recently.

It's strange that the PR's eta was missing. Anyway I created the PR https://github.com/bgub/eta/pull/370.

However, the migration to eta did cause me to learn about the JavaScript with statement that is used by default in both the lodash templating and eta templating engines. It is deprecated: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/with

This improvement to remove the deprecated with can be achieved using eta with [#3513] where the template will use the it. prefix.

[Anonymous]

Originally posted by: jayaddison

@rtritto Ok, yep - you're right that lodash.template is deprecated; I'd forgotten that, and that undermines my argument for staying with it, especially given that it is planned for removal in v5 of lodash: https://github.com/lodash/lodash/blob/a02353279093cca0fea1c8cc468ffbf03bb3485b/threat-model.md#security-notice-_template

Even so: the apostrophe-balance bug in eta suggests to me that that library's parser might not be well-exercised, and it's the kind of bug that could potentially be exploited by specially-crafted template and/or parameter inputs. I don't necessarily think that workbox-build's template would be affected, but by using the library we give it weight as a trusted library, and I'm personally not sure it's ready for that.

I'll add some commentary on the PR you've opened.

(I don't use LLMs for any of my communication, written code, or code review, so please bear with me if I'm a bit slow)

[Anonymous]

Originally posted by: jayaddison

@rtritto did you read about any logicless templating languages, and/or any other logicful templating languages, before selecting eta?

[Anonymous]

Originally posted by: rtritto

@rtritto Ok, yep - you're right that lodash.template is deprecated; I'd forgotten that, and that undermines my argument for staying with it, especially given that it is planned for removal in v5 of lodash: https://github.com/lodash/lodash/blob/a02353279093cca0fea1c8cc468ffbf03bb3485b/threat-model.md#security-notice-_template

So if eta will remain in this repository, can this issue be closed?

Even so: the apostrophe-balance bug in eta suggests to me that that library's parser might not be well-exercised, and it's the kind of bug that could potentially be exploited by specially-crafted template and/or parameter inputs. I don't necessarily think that workbox-build's template would be affected, but by using the library we give it weight as a trusted library, and I'm personally not sure it's ready for that.

I'll add some commentary on the PR you've opened.

Yes, I can support eta with PR, of course on eta repository, you can ping me on issues, open issues or contribute with PRs.

[Anonymous]

Originally posted by: rtritto

@rtritto did you read about any logicless templating languages, and/or any other logicful templating languages, before selecting eta?

I didn't explore further

[Anonymous]

Originally posted by: jayaddison

So if eta will remain in this repository, can this issue be closed?

I'm planning to open a pull request here to offer a migration back to lodash as an alternative -- I'd like to hold this open until I've created that PR and perhaps until it has been reviewed/rejected (or unless I hear from one of the maintainers, or much of the community, that it's not worth doing).

[Anonymous]

Originally posted by: jayaddison

@rtritto did you read about any logicless templating languages, and/or any other logicful templating languages, before selecting eta?

I didn't explore further

Ok. Although they are considered safest, I don't think we could easily use logic-less templating -- like mustache.js (github.com) -- here, because there are a number of if conditions in the template.

Maybe we could refactor it so that the library code conditionally populates variables with each of the JavaScript fragments -- but then the resulting template wouldn't be very easy to audit (the blocks of JavaScript would be in the library code, and might need custom escaping/etc).

However: I do think it would be worth taking some time to compare to other templating engines like EJS and Pug.

Least-dependencies (both runtime and development) is often a really good starting point -- it usually makes everything else easier (and often more high-performance) when there is less code to read and include. So eta already scores well in that area, but I think it is still worth knowing what the other options are like in comparison.