Skip to content

Replace lodash with eta #3489

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.

Sign up for GitHub

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

Merged
swissspidy merged 3 commits into from
Apr 27, 2026
Merged

Replace lodash with eta #3489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bcd3601
replace lodash with eta
rtritto Apr 1, 2026
6454942
fix: renderString call
rtritto Apr 27, 2026
4ce95f0
lint
rtritto Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
"@types/eslint": "^8.56.10",
"@types/estree": "^1.0.5",
"@types/fs-extra": "^9.0.13",
"@types/lodash": "^4.17.0",
"@types/node": "^20.14.8",
"@types/stringify-object": "^3.3.1",
"@typescript-eslint/eslint-plugin": "^5.62.0",
Expand Down
19 changes: 18 additions & 1 deletion packages/workbox-build/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/workbox-build/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@
"@trickfilm400/rollup-plugin-off-main-thread": "^3.0.0-pre1",
"ajv": "^8.6.0",
"common-tags": "^1.8.0",
"eta": "^4.5.1",
"fast-json-stable-stringify": "^2.1.0",
"fs-extra": "^9.0.1",
"glob": "^11.0.1",
"lodash": "^4.17.20",
"pretty-bytes": "^5.3.0",
"rollup": "^4.53.3",
"source-map": "^0.8.0-beta.0",
Expand Down
9 changes: 7 additions & 2 deletions packages/workbox-build/src/lib/populate-sw-template.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
https://opensource.org/licenses/MIT.
*/

import template from 'lodash/template';
import {Eta} from 'eta';

import {errors} from './errors';
import {GeneratePartial, ManifestEntry} from '../types';
Expand All @@ -15,6 +15,11 @@ import {runtimeCachingConverter} from './runtime-caching-converter';
import {stringifyWithoutComments} from './stringify-without-comments';
import {swTemplate} from '../templates/sw-template';

const eta = new Eta({
useWith: true,
Copy link
Copy Markdown

@jayaddison jayaddison May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: I think this relates to the JavaScript with function -- something that can be used by both lodash.template and also eta -- but is deprecated.

Copy link
Copy Markdown
Contributor Author

@rtritto rtritto May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jayaddison how should we change the code?

Copy link
Copy Markdown

@jayaddison jayaddison May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rtritto Personally I'm a bit skeptical about changing the dependency - lodash.template v4.18.0 may have introduced this error (shortly-after resolved in v4.18.1), but aside from that, the dependency itself has been stable for a long time and is used in many places, which to me is usually a sign that it's stable and reliable. Code that doesn't change often can be a good thing.

If there are significant benefits - e.g. much smaller dependency surface and codebase, better stability, auditing/assurance, etc, then perhaps eta would be worth migrating to - but I'm not aware of any significant improvements/drawbacks to back that up.

Neither eta nor lodash.template is a "logic-less templating" engine -- so I'd guess that they're relatively similar in terms of security and functionality.

However: the use of the with statement in JavaScript, highlighted by this eta migration, does make me think that if we move back to lodash.template, then we could explore using the variable option, which means that the evaluated code does not use the with statement.

NB: there was a security bug about that a few years ago, that was subsequently resolved:

...so some caution could be worthwhile.

Copy link
Copy Markdown
Contributor Author

@rtritto rtritto May 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lodash is a generic library with drawbacks of performance and weight where there is an official deprecation of lodash.template.
So it's reasonable to replace it with a library with the specific purpose.
As officially suggested, an alternative is eta.
The useWith: true can be removed if it's not needed. Probably it should be removed with () {} in JavaScript slows down execution and can cause confusing bugs (see https://dev.to/bgub/i-built-a-js-template-engine-3x-faster-than-ejs-lj8).

autoEscape: false,
})

export function populateSWTemplate({
cacheId,
cleanupOutdatedCaches,
Expand Down Expand Up @@ -72,7 +77,7 @@ export function populateSWTemplate({
const moduleRegistry = new ModuleRegistry();

try {
const populatedTemplate = template(swTemplate)({
const populatedTemplate = eta.renderString(swTemplate, {
cacheId,
cleanupOutdatedCaches,
clientsClaim,
Expand Down
194 changes: 106 additions & 88 deletions test/workbox-build/node/lib/populate-sw-template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,14 @@ describe(`[workbox-build] lib/populate-sw-template.js`, function () {
it(`should throw an error if templating fails`, function () {
const manifestEntries = ['ignored'];

// Mock the Eta class and its renderString method to throw an error
const {populateSWTemplate} = proxyquire(MODULE_PATH, {
'lodash/template': () => {
throw new Error();
'eta': {
Eta: class {
renderString() {
throw new Error();
}
},
},
});

Expand All @@ -35,7 +40,11 @@ describe(`[workbox-build] lib/populate-sw-template.js`, function () {

it(`should throw an error if both manifestEntries and runtimeCaching are empty`, function () {
const {populateSWTemplate} = proxyquire(MODULE_PATH, {
'lodash/template': () => {},
'eta': {
Eta: class {
renderString() {}
},
},
});

try {
Expand All @@ -54,10 +63,16 @@ describe(`[workbox-build] lib/populate-sw-template.js`, function () {
const precacheOptionsString = '{}';
const manifestEntries = ['ignored'];

const innerStub = sinon.stub().returns('');
const outerStub = sinon.stub().returns(innerStub);
// Create a single stub to simulate renderString
const renderStringStub = sinon.stub().returns('');
const {populateSWTemplate} = proxyquire(MODULE_PATH, {
'lodash/template': outerStub,
'eta': {
Eta: class {
constructor() {
this.renderString = renderStringStub;
}
},
},
'./runtime-caching-converter': {
runtimeCachingConverter: () => runtimeCachingPlaceholder,
},
Expand All @@ -66,30 +81,30 @@ describe(`[workbox-build] lib/populate-sw-template.js`, function () {

populateSWTemplate({manifestEntries});

expect(outerStub.alwaysCalledWith(swTemplate)).to.be.true;

// Doing a strict comparison with functions isn't easy.
expect(innerStub.args[0][0].use).to.be.a('function');
delete innerStub.args[0][0].use;

expect(innerStub.args[0]).to.eql([
{
manifestEntries,
cacheId: undefined,
cleanupOutdatedCaches: undefined,
clientsClaim: undefined,
disableDevLogs: undefined,
importScripts: undefined,
navigateFallback: undefined,
navigateFallbackDenylist: undefined,
navigateFallbackAllowlist: undefined,
navigationPreload: undefined,
offlineAnalyticsConfigString: undefined,
precacheOptionsString,
runtimeCaching: runtimeCachingPlaceholder,
skipWaiting: undefined,
},
]);
// Eta receives the template as the first argument: args[0][0]
expect(renderStringStub.args[0][0]).to.equal(swTemplate);

// The data is passed as the second argument: args[0][1]
expect(renderStringStub.args[0][1].use).to.be.a('function');
delete renderStringStub.args[0][1].use;

// Compare the data object directly
expect(renderStringStub.args[0][1]).to.eql({
manifestEntries,
cacheId: undefined,
cleanupOutdatedCaches: undefined,
clientsClaim: undefined,
disableDevLogs: undefined,
importScripts: undefined,
navigateFallback: undefined,
navigateFallbackDenylist: undefined,
navigateFallbackAllowlist: undefined,
navigationPreload: undefined,
offlineAnalyticsConfigString: undefined,
precacheOptionsString,
runtimeCaching: runtimeCachingPlaceholder,
skipWaiting: undefined,
});
});

it(`should pass the expected options to the template`, function () {
Expand All @@ -115,14 +130,15 @@ describe(`[workbox-build] lib/populate-sw-template.js`, function () {
const precacheOptionsString =
'{\n "directoryIndex": "index.html",\n "ignoreURLParametersMatching": [/a/, /b/]\n}';

// There are two stages in templating: creating the active template function
// from an initial string, and passing variables to that template function
// to get back a final, populated template string.
// We need to stub out both of those steps to test the full flow.
const templatePopulationStub = sinon.stub().returns('');
const templateCreationStub = sinon.stub().returns(templatePopulationStub);
const renderStringStub = sinon.stub().returns('');
const {populateSWTemplate} = proxyquire(MODULE_PATH, {
'lodash/template': templateCreationStub,
'eta': {
Eta: class {
constructor() {
this.renderString = renderStringStub;
}
},
},
'./runtime-caching-converter': {
runtimeCachingConverter: () => runtimeCachingPlaceholder,
},
Expand All @@ -148,30 +164,30 @@ describe(`[workbox-build] lib/populate-sw-template.js`, function () {
skipWaiting,
});

expect(templateCreationStub.alwaysCalledWith(swTemplate)).to.be.true;

// Doing a strict comparison with functions isn't easy.
expect(templatePopulationStub.args[0][0].use).to.be.a('function');
delete templatePopulationStub.args[0][0].use;

expect(templatePopulationStub.args[0]).to.eql([
{
cacheId,
cleanupOutdatedCaches,
clientsClaim,
disableDevLogs,
importScripts,
manifestEntries,
navigateFallback,
navigateFallbackDenylist,
navigateFallbackAllowlist,
navigationPreload,
offlineAnalyticsConfigString,
runtimeCaching: runtimeCachingPlaceholder,
precacheOptionsString,
skipWaiting,
},
]);
// Eta receives the template as the first argument: args[0][0]
expect(renderStringStub.args[0][0]).to.equal(swTemplate);

// The data is passed as the second argument: args[0][1]
expect(renderStringStub.args[0][1].use).to.be.a('function');
delete renderStringStub.args[0][1].use;

// Compare the data object directly
expect(renderStringStub.args[0][1]).to.eql({
cacheId,
cleanupOutdatedCaches,
clientsClaim,
disableDevLogs,
importScripts,
manifestEntries,
navigateFallback,
navigateFallbackDenylist,
navigateFallbackAllowlist,
navigationPreload,
offlineAnalyticsConfigString,
runtimeCaching: runtimeCachingPlaceholder,
precacheOptionsString,
skipWaiting,
});
});

it(`should handle a complex offlineGoogleAnalytics value when populating the template`, function () {
Expand All @@ -190,10 +206,15 @@ describe(`[workbox-build] lib/populate-sw-template.js`, function () {
const offlineAnalyticsConfigString = `{\n\tparameterOverrides: {\n\t\tcd1: 'offline'\n\t},\n\thitFilter: (params) => {\n \n params.set('cm1', params.get('qt'));\n }\n}`;
const manifestEntries = ['ignored'];

const innerStub = sinon.stub().returns('');
const outerStub = sinon.stub().returns(innerStub);
const renderStringStub = sinon.stub().returns('');
const {populateSWTemplate} = proxyquire(MODULE_PATH, {
'lodash/template': outerStub,
'eta': {
Eta: class {
constructor() {
this.renderString = renderStringStub;
}
},
},
'./runtime-caching-converter': {
runtimeCachingConverter: () => runtimeCachingPlaceholder,
},
Expand All @@ -202,29 +223,26 @@ describe(`[workbox-build] lib/populate-sw-template.js`, function () {

populateSWTemplate({manifestEntries, offlineGoogleAnalytics});

expect(outerStub.alwaysCalledWith(swTemplate)).to.be.true;

// Doing a strict comparison with functions isn't easy.
expect(innerStub.args[0][0].use).to.be.a('function');
delete innerStub.args[0][0].use;

expect(innerStub.args[0]).to.eql([
{
manifestEntries,
cacheId: undefined,
cleanupOutdatedCaches: undefined,
clientsClaim: undefined,
disableDevLogs: undefined,
importScripts: undefined,
navigateFallback: undefined,
navigateFallbackDenylist: undefined,
navigateFallbackAllowlist: undefined,
navigationPreload: undefined,
offlineAnalyticsConfigString,
precacheOptionsString,
runtimeCaching: runtimeCachingPlaceholder,
skipWaiting: undefined,
},
]);
expect(renderStringStub.args[0][0]).to.equal(swTemplate);

expect(renderStringStub.args[0][1].use).to.be.a('function');
delete renderStringStub.args[0][1].use;

expect(renderStringStub.args[0][1]).to.eql({
manifestEntries,
cacheId: undefined,
cleanupOutdatedCaches: undefined,
clientsClaim: undefined,
disableDevLogs: undefined,
importScripts: undefined,
navigateFallback: undefined,
navigateFallbackDenylist: undefined,
navigateFallbackAllowlist: undefined,
navigationPreload: undefined,
offlineAnalyticsConfigString,
precacheOptionsString,
runtimeCaching: runtimeCachingPlaceholder,
skipWaiting: undefined,
});
});
});
Loading