GoogleChrome / GoogleChrome/workbox
Suggestion: revert back to lodash templates, albeit dropping use of the 'with' statement
- Dominant language
- JavaScript
- Stars
- 13k
- Forks
- 880
- Avg merge
- 2h 44m
- Merged PRs (30d)
- 8
Description
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`](https://github.com/lodash/lodash) templating to [`eta`](https://github.com/bgub/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](https://github.com/lodash/lodash/issues/6167#issuecomment-4165130893), 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
Contributor guide
Assessment
This issue has not been assessed yet.