From 99f80da60955496a05a245e52b4c98c92c227fd7 Mon Sep 17 00:00:00 2001 From: Austin McGee <947888+amcgee@users.noreply.github.com> Date: Fri, 16 Sep 2022 12:30:50 +0200 Subject: [PATCH 1/4] fix: support author parsing in d2.config.js --- cli/src/lib/parseConfig.js | 78 +++++++++++++++----------- cli/src/lib/parseConfig.test.js | 81 +++++++++++++++++++++++++++ docs/config/d2-config-js-reference.md | 2 +- 3 files changed, 126 insertions(+), 35 deletions(-) create mode 100644 cli/src/lib/parseConfig.test.js diff --git a/cli/src/lib/parseConfig.js b/cli/src/lib/parseConfig.js index 6de328646..d05236f01 100644 --- a/cli/src/lib/parseConfig.js +++ b/cli/src/lib/parseConfig.js @@ -26,69 +26,79 @@ const parseAuthor = (author) => { const validateConfig = (config) => { if (!requiredConfigFields[config.type]) { - reporter.error( - `Unknown type ${chalk.bold(config.type)} specified in d2.config.js.` + throw new Error( + `Unknown type ${chalk.bold(config.type)} specified in d2.config.js.\n\tValid types: ${Object.keys(requiredConfigFields)}` ) - reporter.error(`\tValid types:`, Object.keys(requiredConfigFields)) - process.exit(1) } requiredConfigFields[config.type].forEach((field) => { if (!has(config, field)) { - reporter.error( + throw new Error( `Required config field ${chalk.bold( field )} not found in d2.config.js or package.json` ) - process.exit(1) } }) return true } +const parseConfigObjects = (config = {}, pkg = {}, { defaultsLib, defaultsApp, defaultsPWA } = {}) => { + const type = config.type || 'app' + reporter.debug(`Type identified : ${chalk.bold(type)}`) + + const defaults = + type === 'lib' ? defaultsLib : defaultsApp + config = defaultsDeep(config, require(defaults)) + + // Add PWA defaults to apps + if (type === 'app') { + config = defaultsDeep(config, require(defaultsPWA)) + } + + config.name = config.name || pkg.name + config.version = config.version || pkg.version + config.author = parseAuthor(config.author || pkg.author) + if (config.author && !config.author.name) { + throw new Error('If author is specified, it must include a valid name') + } + config.description = config.description || pkg.description + config.title = config.title || config.name + + reporter.debug('config loaded', config) + + return config +} + const parseConfig = (paths) => { try { - let config = {} - // if (!fs.existsSync(paths.config)) { - // reporter.error('Config file d2.config.js not found - use the init command to create one!') - // process.exit(1) - // } + let config + let pkg if (fs.existsSync(paths.config)) { reporter.debug('Loading d2 config at', paths.config) config = require(paths.config) reporter.debug('loaded', config) } - - const type = config.type || 'app' - reporter.debug(`Type identified : ${chalk.bold(type)}`) - - const defaults = - type === 'lib' ? paths.configDefaultsLib : paths.configDefaultsApp - config = defaultsDeep(config, require(defaults)) - - // Add PWA defaults to apps - if (type === 'app') { - config = defaultsDeep(config, require(paths.configDefaultsPWA)) - } - if (fs.existsSync(paths.package)) { - const pkg = fs.readJsonSync(paths.package) - config.name = config.name || pkg.name - config.version = config.version || pkg.version - config.author = config.author || parseAuthor(pkg.author) - config.description = config.description || pkg.description + pkg = fs.readJsonSync(paths.package) } - config.title = config.title || config.name - validateConfig(config) + const parsedConfig = parseConfigObjects(config, pkg, { + defaultsLib: paths.configDefaultsLib, + defaultsApp: paths.configDefaultsApp, + defaultsPWA: paths.configDefaultsPWA + }) - reporter.debug('config loaded', config) - return config + validateConfig(parsedConfig) + + return parsedConfig } catch (e) { reporter.error('Failed to load d2 config!') - reporter.debugErr(e) + reporter.error(e) process.exit(1) } } module.exports = parseConfig + +module.exports.parseConfigObjects = parseConfigObjects diff --git a/cli/src/lib/parseConfig.test.js b/cli/src/lib/parseConfig.test.js new file mode 100644 index 000000000..f042e2130 --- /dev/null +++ b/cli/src/lib/parseConfig.test.js @@ -0,0 +1,81 @@ +const { parseConfigObjects } = require('./parseConfig.js') + +describe('parseConfig', () => { + it('should correctly parse author fields', () => { + expect( + parseConfigObjects({ author: { name: 'Test Author' } }).author + ).toMatchObject({ name: 'Test Author' }) + expect( + parseConfigObjects({ + author: { + name: 'Test Author', + email: 'test@author.com', + url: 'https://author.com/test', + }, + }).author + ).toMatchObject({ + name: 'Test Author', + email: 'test@author.com', + url: 'https://author.com/test', + }) + expect( + parseConfigObjects({ + author: { name: 'Test Author', email: 'test@author.com' }, + }).author + ).toMatchObject({ name: 'Test Author', email: 'test@author.com' }) + expect( + parseConfigObjects({ + author: { name: 'Test Author', url: 'https://author.com/test' }, + }).author + ).toMatchObject({ name: 'Test Author', url: 'https://author.com/test' }) + + expect( + parseConfigObjects({ author: 'Test Author' }).author + ).toMatchObject({ name: 'Test Author' }) + expect( + parseConfigObjects({ author: 'Test Author ' }) + .author + ).toMatchObject({ name: 'Test Author', email: 'test@author.com' }) + expect( + parseConfigObjects({ + author: 'Test Author (https://author.com/test)', + }).author + ).toMatchObject({ + name: 'Test Author', + email: 'test@author.com', + url: 'https://author.com/test', + }) + expect( + parseConfigObjects({ + author: 'Test Author (https://author.com/test)', + }).author + ).toMatchObject({ name: 'Test Author', url: 'https://author.com/test' }) + + // missing name + expect( + () => parseConfigObjects({ + author: { + email: 'test@author.com', + url: 'https://author.com/test', + }, + }) + ).toThrow() + expect( + () => parseConfigObjects({ author: { email: 'test@author.com' } }).author + ).toThrow() + expect( + () => parseConfigObjects({ author: { url: 'https://author.com/test' } }) + ).toThrow() + expect( + () => parseConfigObjects({ author: '(https://author.com/test)' }) + ).toThrow() + expect( + () => parseConfigObjects({ + author: ' (https://author.com/test)', + }) + ).toThrow() + expect( + () => parseConfigObjects({ author: '' }) + ).toThrow() + }) +}) diff --git a/docs/config/d2-config-js-reference.md b/docs/config/d2-config-js-reference.md index d4a634448..4d6df1769 100644 --- a/docs/config/d2-config-js-reference.md +++ b/docs/config/d2-config-js-reference.md @@ -18,7 +18,7 @@ The following configuration properties are supported: | **title** | _string_ | `config.name` | The human-readable application title, which will appear in the HeaderBar | | **id** | _string_ | | The ID of the app on the [App Hub](https://apps.dhis2.org/). Used when publishing the app to the App Hub with [d2 app scripts publish](../scripts/publish). See [this guide](https://developers.dhis2.org/docs/guides/publish-apphub/) to learn how to set up continuous delivery. | | **description** | _string_ | `pkg.description` | A full-length description of the application | -| **author** | _string_ | `pkg.author` | The name of the developer to include in the DHIS2 manifest | +| **author** | _string_ of _object_ | `pkg.author` | The name of the developer to include in the DHIS2 manifest, following [package.json author field syntax](https://docs.npmjs.com/cli/v8/configuring-npm/package-json#people-fields-author-contributors). | | **entryPoints.app** | _string_ | **./src/App** | The path to the application entrypoint (not used for libraries) | | **entryPoints.lib** | _string_ or _object_ | **./src/index** | The path to the library entrypoint(s) (not used for applications). Supports [conditional exports](https://nodejs.org/dist/latest-v16.x/docs/api/packages.html#packages_conditional_exports) | | **dataStoreNamespace** | _string_ | | The DataStore and UserDataStore namespace to reserve for this application. The reserved namespace **must** be suitably unique, as other apps will fail to install if they attempt to reserve the same namespace - see the [webapp manifest docs](https://docs.dhis2.org/en/develop/loading-apps.html) | From 066973b1adeedd92cb82dc54b393e3ef45edd878 Mon Sep 17 00:00:00 2001 From: Austin McGee <947888+amcgee@users.noreply.github.com> Date: Fri, 16 Sep 2022 12:34:57 +0200 Subject: [PATCH 2/4] fix: load defaults files before passing to pure function parseConfigObjects --- cli/src/lib/parseConfig.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cli/src/lib/parseConfig.js b/cli/src/lib/parseConfig.js index d05236f01..b2cab58a1 100644 --- a/cli/src/lib/parseConfig.js +++ b/cli/src/lib/parseConfig.js @@ -48,11 +48,11 @@ const parseConfigObjects = (config = {}, pkg = {}, { defaultsLib, defaultsApp, d const defaults = type === 'lib' ? defaultsLib : defaultsApp - config = defaultsDeep(config, require(defaults)) + config = defaultsDeep(config, defaults) // Add PWA defaults to apps if (type === 'app') { - config = defaultsDeep(config, require(defaultsPWA)) + config = defaultsDeep(config, defaultsPWA) } config.name = config.name || pkg.name @@ -84,9 +84,9 @@ const parseConfig = (paths) => { } const parsedConfig = parseConfigObjects(config, pkg, { - defaultsLib: paths.configDefaultsLib, - defaultsApp: paths.configDefaultsApp, - defaultsPWA: paths.configDefaultsPWA + defaultsLib: require(paths.configDefaultsLib), + defaultsApp: require(paths.configDefaultsApp), + defaultsPWA: require(paths.configDefaultsPWA) }) validateConfig(parsedConfig) From 90c86f0068e55bacf30b91d0152e04782923b1a3 Mon Sep 17 00:00:00 2001 From: Austin McGee <947888+amcgee@users.noreply.github.com> Date: Fri, 16 Sep 2022 12:36:02 +0200 Subject: [PATCH 3/4] docs: fix typo --- docs/config/d2-config-js-reference.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/config/d2-config-js-reference.md b/docs/config/d2-config-js-reference.md index 4d6df1769..c924c7de7 100644 --- a/docs/config/d2-config-js-reference.md +++ b/docs/config/d2-config-js-reference.md @@ -18,7 +18,7 @@ The following configuration properties are supported: | **title** | _string_ | `config.name` | The human-readable application title, which will appear in the HeaderBar | | **id** | _string_ | | The ID of the app on the [App Hub](https://apps.dhis2.org/). Used when publishing the app to the App Hub with [d2 app scripts publish](../scripts/publish). See [this guide](https://developers.dhis2.org/docs/guides/publish-apphub/) to learn how to set up continuous delivery. | | **description** | _string_ | `pkg.description` | A full-length description of the application | -| **author** | _string_ of _object_ | `pkg.author` | The name of the developer to include in the DHIS2 manifest, following [package.json author field syntax](https://docs.npmjs.com/cli/v8/configuring-npm/package-json#people-fields-author-contributors). | +| **author** | _string_ or _object_ | `pkg.author` | The name of the developer to include in the DHIS2 manifest, following [package.json author field syntax](https://docs.npmjs.com/cli/v8/configuring-npm/package-json#people-fields-author-contributors). | | **entryPoints.app** | _string_ | **./src/App** | The path to the application entrypoint (not used for libraries) | | **entryPoints.lib** | _string_ or _object_ | **./src/index** | The path to the library entrypoint(s) (not used for applications). Supports [conditional exports](https://nodejs.org/dist/latest-v16.x/docs/api/packages.html#packages_conditional_exports) | | **dataStoreNamespace** | _string_ | | The DataStore and UserDataStore namespace to reserve for this application. The reserved namespace **must** be suitably unique, as other apps will fail to install if they attempt to reserve the same namespace - see the [webapp manifest docs](https://docs.dhis2.org/en/develop/loading-apps.html) | From 30354568d0f93dbf69a598f68b6ab92f1f07db28 Mon Sep 17 00:00:00 2001 From: Austin McGee <947888+amcgee@users.noreply.github.com> Date: Fri, 16 Sep 2022 12:42:27 +0200 Subject: [PATCH 4/4] style: formatting --- cli/src/lib/parseConfig.js | 17 ++++++++++++----- cli/src/lib/parseConfig.test.js | 24 +++++++++++++----------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/cli/src/lib/parseConfig.js b/cli/src/lib/parseConfig.js index b2cab58a1..9d5f7c5cf 100644 --- a/cli/src/lib/parseConfig.js +++ b/cli/src/lib/parseConfig.js @@ -27,7 +27,11 @@ const parseAuthor = (author) => { const validateConfig = (config) => { if (!requiredConfigFields[config.type]) { throw new Error( - `Unknown type ${chalk.bold(config.type)} specified in d2.config.js.\n\tValid types: ${Object.keys(requiredConfigFields)}` + `Unknown type ${chalk.bold( + config.type + )} specified in d2.config.js.\n\tValid types: ${Object.keys( + requiredConfigFields + )}` ) } requiredConfigFields[config.type].forEach((field) => { @@ -42,12 +46,15 @@ const validateConfig = (config) => { return true } -const parseConfigObjects = (config = {}, pkg = {}, { defaultsLib, defaultsApp, defaultsPWA } = {}) => { +const parseConfigObjects = ( + config = {}, + pkg = {}, + { defaultsLib, defaultsApp, defaultsPWA } = {} +) => { const type = config.type || 'app' reporter.debug(`Type identified : ${chalk.bold(type)}`) - const defaults = - type === 'lib' ? defaultsLib : defaultsApp + const defaults = type === 'lib' ? defaultsLib : defaultsApp config = defaultsDeep(config, defaults) // Add PWA defaults to apps @@ -86,7 +93,7 @@ const parseConfig = (paths) => { const parsedConfig = parseConfigObjects(config, pkg, { defaultsLib: require(paths.configDefaultsLib), defaultsApp: require(paths.configDefaultsApp), - defaultsPWA: require(paths.configDefaultsPWA) + defaultsPWA: require(paths.configDefaultsPWA), }) validateConfig(parsedConfig) diff --git a/cli/src/lib/parseConfig.test.js b/cli/src/lib/parseConfig.test.js index f042e2130..d6a3d9c67 100644 --- a/cli/src/lib/parseConfig.test.js +++ b/cli/src/lib/parseConfig.test.js @@ -52,8 +52,8 @@ describe('parseConfig', () => { ).toMatchObject({ name: 'Test Author', url: 'https://author.com/test' }) // missing name - expect( - () => parseConfigObjects({ + expect(() => + parseConfigObjects({ author: { email: 'test@author.com', url: 'https://author.com/test', @@ -61,21 +61,23 @@ describe('parseConfig', () => { }) ).toThrow() expect( - () => parseConfigObjects({ author: { email: 'test@author.com' } }).author + () => + parseConfigObjects({ author: { email: 'test@author.com' } }) + .author ).toThrow() - expect( - () => parseConfigObjects({ author: { url: 'https://author.com/test' } }) + expect(() => + parseConfigObjects({ author: { url: 'https://author.com/test' } }) ).toThrow() - expect( - () => parseConfigObjects({ author: '(https://author.com/test)' }) + expect(() => + parseConfigObjects({ author: '(https://author.com/test)' }) ).toThrow() - expect( - () => parseConfigObjects({ + expect(() => + parseConfigObjects({ author: ' (https://author.com/test)', }) ).toThrow() - expect( - () => parseConfigObjects({ author: '' }) + expect(() => + parseConfigObjects({ author: '' }) ).toThrow() }) })