Background
The labsEnabledHelpers object in lib/checker.js has been an empty inline object for ~4.5 years, making the associated logic inert. In PR #723, it was extracted to lib/utils/labs-enabled-helpers.js as a scaffold for future use. This means the bugs in the existing logic will become active as soon as the first entry is added to that map.
Bugs identified
1. Spec mutation persists across check() calls within the same process
The loop mutates spec.knownHelpers directly:
spec.knownHelpers.push(helper);
Since the spec object is shared/cached, a helper enabled in one call remains enabled in all subsequent calls in the same process, even if the labs flag is absent.
2. _.has() checks key existence, not truthiness
if (_.has(options.labs, flag)) {This means { labs: { testFlag: false } } would incorrectly enable the helper. The check should use a truthy test (e.g., options.labs && options.labs[flag]).
Suggested fix
Compute the list of extra helpers per invocation using a truthy check and merge it transiently without mutating spec.knownHelpers.
Options to consider
- Fix both bugs and add regression tests.
- Remove
labsEnabledHelpers entirely if it's no longer needed.
- Find a different documentation pattern for labs-flagged helpers.
References
Background
The
labsEnabledHelpersobject inlib/checker.jshas been an empty inline object for ~4.5 years, making the associated logic inert. In PR #723, it was extracted tolib/utils/labs-enabled-helpers.jsas a scaffold for future use. This means the bugs in the existing logic will become active as soon as the first entry is added to that map.Bugs identified
1. Spec mutation persists across
check()calls within the same processThe loop mutates
spec.knownHelpersdirectly:Since the spec object is shared/cached, a helper enabled in one call remains enabled in all subsequent calls in the same process, even if the
labsflag is absent.2.
_.has()checks key existence, not truthinessThis means
{ labs: { testFlag: false } }would incorrectly enable the helper. The check should use a truthy test (e.g.,options.labs && options.labs[flag]).Suggested fix
Compute the list of extra helpers per invocation using a truthy check and merge it transiently without mutating
spec.knownHelpers.Options to consider
labsEnabledHelpersentirely if it's no longer needed.References