From 3e93fe1bd4ce0776fd517fc2eed232592d707afe Mon Sep 17 00:00:00 2001 From: Jack Murray Date: Tue, 23 Jan 2024 10:56:53 +0000 Subject: [PATCH 1/3] add a failing test to repro the problem Signed-off-by: Jack Murray <115712715+jackmtpt@users.noreply.github.com> --- ...cs_with_additional_plugins_with_config.yml | 9 ++++++ .../src/stages/generate/helpers.test.ts | 30 +++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 plugins/techdocs-node/src/stages/generate/__fixtures__/mkdocs_with_additional_plugins_with_config.yml diff --git a/plugins/techdocs-node/src/stages/generate/__fixtures__/mkdocs_with_additional_plugins_with_config.yml b/plugins/techdocs-node/src/stages/generate/__fixtures__/mkdocs_with_additional_plugins_with_config.yml new file mode 100644 index 0000000000..6c945162d7 --- /dev/null +++ b/plugins/techdocs-node/src/stages/generate/__fixtures__/mkdocs_with_additional_plugins_with_config.yml @@ -0,0 +1,9 @@ +site_name: Test site name +site_description: Test site description +docs_dir: docs/ +plugins: + - not-techdocs-core + - also-not-techdocs-core + - custom-plugin: + with: + configuration: 1 diff --git a/plugins/techdocs-node/src/stages/generate/helpers.test.ts b/plugins/techdocs-node/src/stages/generate/helpers.test.ts index 24d9b3effb..1b47cfb05c 100644 --- a/plugins/techdocs-node/src/stages/generate/helpers.test.ts +++ b/plugins/techdocs-node/src/stages/generate/helpers.test.ts @@ -81,6 +81,12 @@ const mkdocsYmlWithoutPlugins = fs.readFileSync( const mkdocsYmlWithAdditionalPlugins = fs.readFileSync( resolvePath(__filename, '../__fixtures__/mkdocs_with_additional_plugins.yml'), ); +const mkdocsYmlWithAdditionalPluginsWithConfig = fs.readFileSync( + resolvePath( + __filename, + '../__fixtures__/mkdocs_with_additional_plugins_with_config.yml', + ), +); const mkdocsYmlWithEnvTag = fs.readFileSync( resolvePath(__filename, '../__fixtures__/mkdocs_with_env_tag.yml'), ); @@ -321,6 +327,8 @@ describe('helpers', () => { 'mkdocs_with_techdocs_plugin.yml': mkdocsYmlWithTechdocsPlugins, 'mkdocs_without_plugins.yml': mkdocsYmlWithoutPlugins, 'mkdocs_with_additional_plugins.yml': mkdocsYmlWithAdditionalPlugins, + 'mkdocs_with_additional_plugins_with_config.yml': + mkdocsYmlWithAdditionalPluginsWithConfig, }); }); it('should not add additional plugins if techdocs exists already in mkdocs file', async () => { @@ -386,6 +394,28 @@ describe('helpers', () => { expect(parsedYml.plugins).toContain('techdocs-core'); expect(parsedYml.plugins).toContain('custom-plugin'); }); + it('should not overwrite config when defaults are added', async () => { + await patchMkdocsYmlWithPlugins( + mockDir.resolve('mkdocs_with_additional_plugins_with_config.yml'), + mockLogger, + ['techdocs-core', 'custom-plugin'], + ); + + const updatedMkdocsYml = await fs.readFile( + mockDir.resolve('mkdocs_with_additional_plugins_with_config.yml'), + ); + const parsedYml = yaml.load(updatedMkdocsYml.toString()) as { + plugins: object[]; + }; + expect(parsedYml.plugins).toHaveLength(4); + expect(parsedYml.plugins).toContain('techdocs-core'); + // we want our original object with its properties to be preserved, and for the basic string form of the plugin + // to NOT be added as well. + expect(parsedYml.plugins).not.toContain('custom-plugin'); + expect(parsedYml.plugins).toContainEqual({ + 'custom-plugin': { with: { configuration: 1 } }, + }); + }); }); describe('patchIndexPreBuild', () => { From bdbbd096a681ef20264b6710849905e794e55560 Mon Sep 17 00:00:00 2001 From: Jack Murray Date: Tue, 23 Jan 2024 12:38:29 +0000 Subject: [PATCH 2/3] rewrite patchMkdocsYmlWithPlugins to correctly only merge in all default plugins that are missing Signed-off-by: Jack Murray <115712715+jackmtpt@users.noreply.github.com> --- .../src/stages/generate/mkdocsPatchers.ts | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/plugins/techdocs-node/src/stages/generate/mkdocsPatchers.ts b/plugins/techdocs-node/src/stages/generate/mkdocsPatchers.ts index 945aab3884..762a1c267a 100644 --- a/plugins/techdocs-node/src/stages/generate/mkdocsPatchers.ts +++ b/plugins/techdocs-node/src/stages/generate/mkdocsPatchers.ts @@ -153,21 +153,30 @@ export const patchMkdocsYmlWithPlugins = async ( defaultPlugins: string[] = ['techdocs-core'], ) => { await patchMkdocsFile(mkdocsYmlPath, logger, mkdocsYml => { - // Modify mkdocs.yaml to contain the required default plugins + // Modify mkdocs.yaml to contain the required default plugins. + // If no plugins are defined we can just return the defaults. if (!('plugins' in mkdocsYml)) { mkdocsYml.plugins = defaultPlugins; return true; } - if ( - mkdocsYml.plugins && - !defaultPlugins.every(plugin => mkdocsYml.plugins!.includes(plugin)) - ) { - mkdocsYml.plugins = [ - ...new Set([...mkdocsYml.plugins, ...defaultPlugins]), - ]; - return true; - } - return false; + // Otherwise, check each default plugin and include it if necessary. + let changesMade = false; + + defaultPlugins.forEach(dp => { + // if the plugin isn't there as a string, and isn't there as an object (which may itself contain extra config) + // then we need to add it + if ( + !( + mkdocsYml.plugins!.includes(dp) || + mkdocsYml.plugins!.some(p => p.hasOwnProperty(dp)) + ) + ) { + mkdocsYml.plugins = [...new Set([...mkdocsYml.plugins!, dp])]; + changesMade = true; + } + }); + + return changesMade; }); }; From 5b4f565a1c68801158b61f974e53f134b13b2039 Mon Sep 17 00:00:00 2001 From: Jack Murray <115712715+jackmtpt@users.noreply.github.com> Date: Mon, 19 Feb 2024 11:35:52 +0000 Subject: [PATCH 3/3] add changeset Signed-off-by: Jack Murray <115712715+jackmtpt@users.noreply.github.com> --- .changeset/fresh-rings-tell.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fresh-rings-tell.md diff --git a/.changeset/fresh-rings-tell.md b/.changeset/fresh-rings-tell.md new file mode 100644 index 0000000000..82fe952e4a --- /dev/null +++ b/.changeset/fresh-rings-tell.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-techdocs-node': patch +--- + +Fix handling of default plugins that have configuration