From b8ff77ca3cf0ef9deeab44ae4095c5698dde2920 Mon Sep 17 00:00:00 2001 From: flakey5 <73616808+flakey5@users.noreply.github.com> Date: Fri, 28 Nov 2025 14:18:34 -0800 Subject: [PATCH 1/2] fix(legacy-json): misc promotion logic fix Re https://github.com/nodejs/node/pull/57343#issuecomment-3583378011 Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com> --- src/generators/legacy-json/utils/buildSection.mjs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/generators/legacy-json/utils/buildSection.mjs b/src/generators/legacy-json/utils/buildSection.mjs index f70c3de3..56ca35c6 100644 --- a/src/generators/legacy-json/utils/buildSection.mjs +++ b/src/generators/legacy-json/utils/buildSection.mjs @@ -116,11 +116,11 @@ export const createSectionBuilder = () => { // Only promote certain keys if (!UNPROMOTED_KEYS.includes(key)) { // Merge the section's properties into the parent section - parent[key] = parent[key] - ? // If the parent already has this key, concatenate the values - [].concat(parent[key], value) - : // Otherwise, directly assign the section's value to the parent - []; + if (parent[key] && Array.isArray(parent[key])) { + parent[key] = parent[key].concat(value); + } else { + parent[key] ||= value; + } } }); } From bcb615e215c33738cd70e30f581c92cf6a21fbde Mon Sep 17 00:00:00 2001 From: flakey5 <73616808+flakey5@users.noreply.github.com> Date: Fri, 28 Nov 2025 15:34:53 -0800 Subject: [PATCH 2/2] unit tests Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com> --- .../utils/__tests__/buildSection.test.mjs | 138 ++++++++++++++++++ .../legacy-json/utils/buildSection.mjs | 44 +++--- 2 files changed, 160 insertions(+), 22 deletions(-) create mode 100644 src/generators/legacy-json/utils/__tests__/buildSection.test.mjs diff --git a/src/generators/legacy-json/utils/__tests__/buildSection.test.mjs b/src/generators/legacy-json/utils/__tests__/buildSection.test.mjs new file mode 100644 index 00000000..71eeadc3 --- /dev/null +++ b/src/generators/legacy-json/utils/__tests__/buildSection.test.mjs @@ -0,0 +1,138 @@ +'use strict'; + +import assert from 'node:assert/strict'; +import { describe, test } from 'node:test'; + +import { UNPROMOTED_KEYS } from '../../constants.mjs'; +import { promoteMiscChildren } from '../buildSection.mjs'; + +describe('promoteMiscChildren', () => { + /** + * @template {object} T + * + * @param {T} base + * @param {'section'|'parent'} [type='section'] + * @returns {T} + */ + function buildReadOnlySection(base, type = 'section') { + return new Proxy(base, { + set(_, key) { + throw new Error(`${type} property '${String(key)} modified`); + }, + }); + } + + test('ignores non-misc section', () => { + const section = buildReadOnlySection({ + type: 'text', + }); + + const parent = buildReadOnlySection( + { + type: 'text', + }, + 'parent' + ); + + promoteMiscChildren(section, parent); + }); + + test('ignores misc parent', () => { + const section = buildReadOnlySection({ + type: 'misc', + }); + + const parent = buildReadOnlySection( + { + type: 'misc', + }, + 'parent' + ); + + promoteMiscChildren(section, parent); + }); + + test('ignores keys in UNPROMOTED_KEYS', () => { + const sectionRaw = { + type: 'misc', + promotableKey: 'this should be promoted', + }; + + UNPROMOTED_KEYS.forEach(key => { + if (key === 'type') { + return; + } + + sectionRaw[key] = 'this should be ignored'; + }); + + const section = buildReadOnlySection(sectionRaw); + + const parent = { + type: 'module', + }; + + promoteMiscChildren(section, parent); + + UNPROMOTED_KEYS.forEach(key => { + if (key === 'type') { + return; + } + + if (parent[key]) { + throw new Error(`'${key}' was promoted`); + } + }); + + assert.strictEqual(parent.promotableKey, section.promotableKey); + }); + + describe('merges properties correctly', () => { + test('pushes child property if parent is an array', () => { + const section = buildReadOnlySection({ + type: 'misc', + someValue: 'bar', + }); + + const parent = { + type: 'module', + someValue: ['foo'], + }; + + promoteMiscChildren(section, parent); + + assert.deepStrictEqual(parent.someValue, ['foo', 'bar']); + }); + + test('ignores child property if parent has a value that is not an array', () => { + const section = buildReadOnlySection({ + type: 'misc', + someValue: 'bar', + }); + + const parent = { + type: 'module', + someValue: 'foo', + }; + + promoteMiscChildren(section, parent); + + assert.strictEqual(parent.someValue, 'foo'); + }); + + test('promotes child property if parent does not have the property', () => { + const section = buildReadOnlySection({ + type: 'misc', + someValue: 'bar', + }); + + const parent = { + type: 'module', + }; + + promoteMiscChildren(section, parent); + + assert.deepStrictEqual(parent.someValue, 'bar'); + }); + }); +}); diff --git a/src/generators/legacy-json/utils/buildSection.mjs b/src/generators/legacy-json/utils/buildSection.mjs index 56ca35c6..4f647961 100644 --- a/src/generators/legacy-json/utils/buildSection.mjs +++ b/src/generators/legacy-json/utils/buildSection.mjs @@ -5,6 +5,28 @@ import { getRemarkRehype } from '../../../utils/remark.mjs'; import { transformNodesToString } from '../../../utils/unist.mjs'; import { SECTION_TYPE_PLURALS, UNPROMOTED_KEYS } from '../constants.mjs'; +/** + * Promotes children properties to the parent level if the section type is 'misc'. + * @param {import('../types.d.ts').Section} section - The section to promote. + * @param {import('../types.d.ts').Section} parent - The parent section. + */ +export const promoteMiscChildren = (section, parent) => { + // Only promote if the current section is of type 'misc' and the parent is not 'misc' + if (section.type === 'misc' && parent.type !== 'misc') { + Object.entries(section).forEach(([key, value]) => { + // Only promote certain keys + if (!UNPROMOTED_KEYS.includes(key)) { + // Merge the section's properties into the parent section + if (parent[key] && Array.isArray(parent[key])) { + parent[key] = parent[key].concat(value); + } else { + parent[key] ||= value; + } + } + }); + } +}; + /** * */ @@ -104,28 +126,6 @@ export const createSectionBuilder = () => { parent[key].push(section); }; - /** - * Promotes children properties to the parent level if the section type is 'misc'. - * @param {import('../types.d.ts').Section} section - The section to promote. - * @param {import('../types.d.ts').Section} parent - The parent section. - */ - const promoteMiscChildren = (section, parent) => { - // Only promote if the current section is of type 'misc' and the parent is not 'misc' - if (section.type === 'misc' && parent.type !== 'misc') { - Object.entries(section).forEach(([key, value]) => { - // Only promote certain keys - if (!UNPROMOTED_KEYS.includes(key)) { - // Merge the section's properties into the parent section - if (parent[key] && Array.isArray(parent[key])) { - parent[key] = parent[key].concat(value); - } else { - parent[key] ||= value; - } - } - }); - } - }; - /** * Processes children of a given entry and updates the section. * @param {import('../types.d.ts').HierarchizedEntry} entry - The current entry.