-
Notifications
You must be signed in to change notification settings - Fork 4
feat(config): allow external config files in js/ts format #68
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
const path = require('path'); | ||
const inquirer = require('inquirer'); | ||
const { createJiti } = require('jiti'); | ||
const mergeOptions = require('merge-options').bind({ ignoreUndefined: true }); | ||
const { cosmiconfig } = require('cosmiconfig'); | ||
|
||
|
@@ -11,6 +12,8 @@ const STORAGE_CONTENT = 'content'; | |
const STATE_SUCCESS = 'success'; | ||
const STATE_FAILURE = 'failure'; | ||
|
||
const jiti = createJiti(__filename); | ||
|
||
/** | ||
* Get configuration | ||
* @param {Object} args | ||
|
@@ -26,7 +29,7 @@ const getConfig = async (args) => { | |
const environmentOptions = { | ||
spaceId: process.env.CONTENTFUL_SPACE_ID, | ||
environmentId: process.env.CONTENTFUL_ENVIRONMENT_ID, | ||
accessToken: process.env.CONTENTFUL_MANAGEMENT_TOKEN, | ||
managementToken: process.env.CONTENTFUL_MANAGEMENT_TOKEN, | ||
host: process.env.CONTENTFUL_HOST, | ||
}; | ||
|
||
|
@@ -40,7 +43,7 @@ const getConfig = async (args) => { | |
const { managementToken, activeSpaceId, activeEnvironmentId, host } = config || {}; | ||
contentfulCliOptions = { | ||
spaceId: activeSpaceId, | ||
accessToken: managementToken, | ||
managementToken: managementToken, | ||
host, | ||
}; | ||
} | ||
|
@@ -52,7 +55,7 @@ const getConfig = async (args) => { | |
try { | ||
// get configuration from migrations rc file | ||
const explorer = cosmiconfig('migrations'); | ||
const explorerResult = args.config ? await explorer.load(args.config) : await explorer.search(); | ||
const explorerResult = await explorer.search(); | ||
if (explorerResult !== null) { | ||
const { config, filepath } = explorerResult || {}; | ||
|
||
|
@@ -69,7 +72,53 @@ const getConfig = async (args) => { | |
console.log('Error:', error.message); | ||
} | ||
|
||
return mergeOptions(defaultOptions, contentfulCliOptions, environmentOptions, configFileOptions, args || {}); | ||
let userConfigFileOptions = {}; | ||
if (args.config) { | ||
try { | ||
let config = {}; | ||
let filepath = ''; | ||
if (args.config.endsWith('.js') || args.config.endsWith('.ts')) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When we now support ts files, that must be documented in the readme |
||
config = await jiti.import(path.resolve(args.config)); | ||
filepath = jiti.esmResolve(path.resolve(args.config)); | ||
} else { | ||
const explorer = cosmiconfig('migrations'); | ||
const explorerResult = await explorer.load(args.config); | ||
config = explorerResult.config; | ||
filepath = explorerResult.filepath; | ||
} | ||
if (config) { | ||
userConfigFileOptions = { | ||
directory: path.resolve(path.dirname(filepath || ''), args.directory || 'migrations'), | ||
...(config || {}), | ||
}; | ||
|
||
if (userConfigFileOptions.directory && !path.isAbsolute(userConfigFileOptions.directory)) { | ||
userConfigFileOptions.directory = path.resolve(path.dirname(filepath || ''), userConfigFileOptions.directory); | ||
} | ||
Comment on lines
+90
to
+97
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This whole getConfig function got quite long and is hard to digest. |
||
|
||
if (configFileOptions.directory) { | ||
delete userConfigFileOptions.directory; | ||
} | ||
Comment on lines
+99
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not clear why the config directory takes precedence over the user config directory. Would be nice to have a comment that explains that. |
||
} | ||
} catch (error) { | ||
console.log('Error:', error.message); | ||
} | ||
} | ||
|
||
const result = mergeOptions( | ||
defaultOptions, | ||
contentfulCliOptions, | ||
environmentOptions, | ||
configFileOptions, | ||
userConfigFileOptions, | ||
args || {} | ||
); | ||
|
||
if (result.managementToken) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to be done to help with compatibility. |
||
result.accessToken = result.managementToken; | ||
} | ||
|
||
return result; | ||
}; | ||
|
||
const getPromts = (data) => { | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,7 @@ | |
"author": "Ben Zörb <benjamin.zoerb@jvm.de>", | ||
"license": "MIT", | ||
"engines": { | ||
"node": ">=14" | ||
"node": ">=18" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to increment the version with all those changes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, would be minor but as we increased the required node version this needs to be major. Version will be bumped in the release process |
||
}, | ||
"keywords": [ | ||
"contentful", | ||
|
@@ -51,6 +51,7 @@ | |
"fs-extra": "^10.1.0", | ||
"globby": "^12.0.2", | ||
"inquirer": "^8.2.0", | ||
"jiti": "^2.4.2", | ||
"markdown-table": "^3.0.4", | ||
"merge-options": "^3.0.4", | ||
"mustache": "^4.2.0", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably also should be documented, that we now expect managementToken as the name (but still support accessToken?)