-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: new switch v2 function #20
base: develop
Are you sure you want to change the base?
Changes from 2 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 |
---|---|---|
@@ -0,0 +1,18 @@ | ||
/** | ||
* @fileOverview This is a variation of a SailsJS local.js file that uses env-lift to provide configuration overrides | ||
* from environment variables. | ||
*/ | ||
module.exports = require('../../index').switchV2(__filename, { | ||
port: 1337, | ||
|
||
environment: 'development', | ||
|
||
models: { | ||
connection: 'mySQL', | ||
migrate: 'safe' | ||
}, | ||
|
||
log: { | ||
level: 'info' | ||
} | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
/** | ||
* @fileOverview This is a variation of a SailsJS local.js file that uses env-lift to provide configuration overrides | ||
* from environment variables. | ||
*/ | ||
module.exports = require('../../index').switchV2(__filename, { | ||
port: 6060, | ||
|
||
environment: 'v2-internal', | ||
|
||
log: { | ||
level: 'silly' | ||
}, | ||
|
||
foo: { | ||
bar: 'v2-dummy' | ||
} | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
var path = require('path'); | ||
const path = require('path'); | ||
|
||
module.exports = { | ||
/** | ||
|
@@ -42,7 +42,7 @@ module.exports = { | |
} | ||
|
||
// Get the file name from which config will be loaded. | ||
var targetFile = require.resolve(path.join(process.cwd(), 'config', 'env', process.env.SAILS_ENV)); | ||
const targetFile = require.resolve(path.join(process.cwd(), 'config', 'env', process.env.SAILS_ENV)); | ||
|
||
// if the parent file name from which this module is called is same as the targetFile name then no need | ||
// to require the targetFile as we are already at the target File and we need that file's config. | ||
|
@@ -57,6 +57,45 @@ module.exports = { | |
// called and that is dynamic, so we need to make sure we are requiring the correct parent file everytime. | ||
delete require.cache[__filename]; | ||
|
||
// Finally return the target file's config. | ||
return require(targetFile); | ||
}, | ||
Comment on lines
+61
to
+63
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. Wasn't this already present ? Why showing up as newly added 😕 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. GitHub diff is showing lines 92 and below as existing. |
||
|
||
/** | ||
* `delete require.cache()` might not work properly if there is any hook applied to require module, eg: require-in-the-middle | ||
* switchV2() will expect parentFilename as an parameter and use that to compare with the targetFile. | ||
* | ||
* It will pick the env config based on the SAILS_ENV value. If a project has SAILS_ENV | ||
* defined to some value then it expects that same file in the env/config file so that it | ||
* will pick the config value from that file. | ||
* | ||
* @param env this is the current sails environment in which this module is used. | ||
* @param parentFilename this is the filename of the module which is calling switchV2. | ||
* @param defaultConfig this is the default config which is bases on NODE_ENV value | ||
* @returns {config} | ||
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. For any other reviewers, |
||
* | ||
* @example | ||
* // This is the env config file | ||
* module.exports = require('sails-env-switch').switchV2(__filename, { | ||
* port: 8080, | ||
* name: 'Sample' | ||
* }); | ||
*/ | ||
switchV2: function (parentFilename, defaultConfig) { | ||
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. Imo the stack approach is better, this we can keep the interface same. If need be switch and switchV2 can be merged. cc: @kunagpal |
||
// If SAILS_ENV is not defined, return the default config | ||
if (!process.env.SAILS_ENV) { | ||
return defaultConfig; | ||
} | ||
|
||
// Get the file name from which config will be loaded. | ||
const targetFile = require.resolve(path.join(process.cwd(), 'config', 'env', process.env.SAILS_ENV)); | ||
|
||
// if the parent file name is same as the targetFile name then no need | ||
// to require the targetFile as we are already at the target File and we need that file's config. | ||
if (path.relative(targetFile, parentFilename) === '') { | ||
return defaultConfig; | ||
} | ||
|
||
// Finally return the target file's config. | ||
return require(targetFile); | ||
} | ||
|
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.
revert ? let's not make unnecessary changes