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
Add support of serverless.js configuration file #4590
Changes from 2 commits
bca4da5
ea7c662
d05e456
d2ed158
a2ae114
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 |
---|---|---|
|
@@ -46,6 +46,7 @@ class Service { | |
'serverless.yaml', | ||
'serverless.yml', | ||
'serverless.json', | ||
'serverless.js', | ||
]; | ||
|
||
const serviceFilePaths = _.map(serviceFilenames, filename => path.join(servicePath, filename)); | ||
|
@@ -61,65 +62,79 @@ class Service { | |
serviceFilenames[serviceFileIndex] : | ||
_.first(serviceFilenames); | ||
|
||
if (serviceFilename === 'serverless.js') { | ||
return new BbPromise((resolve) => { | ||
// use require to load serverless.js file | ||
// eslint-disable-next-line global-require | ||
resolve(that.loadServiceFileParam(serviceFilename, require(serviceFilePath))); | ||
}); | ||
} | ||
|
||
return that.serverless.yamlParser | ||
.parse(serviceFilePath) | ||
.then((serverlessFileParam) => { | ||
const serverlessFile = serverlessFileParam; | ||
// basic service level validation | ||
const version = this.serverless.utils.getVersion(); | ||
const ymlVersion = serverlessFile.frameworkVersion; | ||
if (ymlVersion && !semver.satisfies(version, ymlVersion)) { | ||
const errorMessage = [ | ||
`The Serverless version (${version}) does not satisfy the`, | ||
` "frameworkVersion" (${ymlVersion}) in ${serviceFilename}`, | ||
].join(''); | ||
throw new ServerlessError(errorMessage); | ||
} | ||
if (!serverlessFile.service) { | ||
throw new ServerlessError(`"service" property is missing in ${serviceFilename}`); | ||
} | ||
if (_.isObject(serverlessFile.service) && !serverlessFile.service.name) { | ||
throw new ServerlessError(`"service" is missing the "name" property in ${serviceFilename}`); // eslint-disable-line max-len | ||
} | ||
if (!serverlessFile.provider) { | ||
throw new ServerlessError(`"provider" property is missing in ${serviceFilename}`); | ||
} | ||
.then((serverlessFileParam) => | ||
that.loadServiceFileParam(serviceFilename, serverlessFileParam) | ||
); | ||
} | ||
|
||
if (typeof serverlessFile.provider !== 'object') { | ||
const providerName = serverlessFile.provider; | ||
serverlessFile.provider = { | ||
name: providerName, | ||
}; | ||
} | ||
loadServiceFileParam(serviceFilename, serverlessFileParam) { | ||
const that = this; | ||
|
||
if (_.isObject(serverlessFile.service)) { | ||
that.serviceObject = serverlessFile.service; | ||
that.service = serverlessFile.service.name; | ||
} else { | ||
that.serviceObject = { name: serverlessFile.service }; | ||
that.service = serverlessFile.service; | ||
} | ||
const serverlessFile = serverlessFileParam; | ||
// basic service level validation | ||
const version = this.serverless.utils.getVersion(); | ||
const ymlVersion = serverlessFile.frameworkVersion; | ||
if (ymlVersion && !semver.satisfies(version, ymlVersion)) { | ||
const errorMessage = [ | ||
`The Serverless version (${version}) does not satisfy the`, | ||
` "frameworkVersion" (${ymlVersion}) in ${serviceFilename}`, | ||
].join(''); | ||
throw new ServerlessError(errorMessage); | ||
} | ||
if (!serverlessFile.service) { | ||
throw new ServerlessError(`"service" property is missing in ${serviceFilename}`); | ||
} | ||
if (_.isObject(serverlessFile.service) && !serverlessFile.service.name) { | ||
throw new ServerlessError(`"service" is missing the "name" property in ${serviceFilename}`); // eslint-disable-line max-len | ||
} | ||
if (!serverlessFile.provider) { | ||
throw new ServerlessError(`"provider" property is missing in ${serviceFilename}`); | ||
} | ||
|
||
that.custom = serverlessFile.custom; | ||
that.plugins = serverlessFile.plugins; | ||
that.resources = serverlessFile.resources; | ||
that.functions = serverlessFile.functions || {}; | ||
|
||
// merge so that the default settings are still in place and | ||
// won't be overwritten | ||
that.provider = _.merge(that.provider, serverlessFile.provider); | ||
|
||
if (serverlessFile.package) { | ||
that.package.individually = serverlessFile.package.individually; | ||
that.package.path = serverlessFile.package.path; | ||
that.package.artifact = serverlessFile.package.artifact; | ||
that.package.exclude = serverlessFile.package.exclude; | ||
that.package.include = serverlessFile.package.include; | ||
that.package.excludeDevDependencies = serverlessFile.package.excludeDevDependencies; | ||
} | ||
if (typeof serverlessFile.provider !== 'object') { | ||
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 should consistently use lodash:
|
||
const providerName = serverlessFile.provider; | ||
serverlessFile.provider = { | ||
name: providerName, | ||
}; | ||
} | ||
|
||
return this; | ||
}); | ||
if (_.isObject(serverlessFile.service)) { | ||
that.serviceObject = serverlessFile.service; | ||
that.service = serverlessFile.service.name; | ||
} else { | ||
that.serviceObject = { name: serverlessFile.service }; | ||
that.service = serverlessFile.service; | ||
} | ||
|
||
that.custom = serverlessFile.custom; | ||
that.plugins = serverlessFile.plugins; | ||
that.resources = serverlessFile.resources; | ||
that.functions = serverlessFile.functions || {}; | ||
|
||
// merge so that the default settings are still in place and | ||
// won't be overwritten | ||
that.provider = _.merge(that.provider, serverlessFile.provider); | ||
|
||
if (serverlessFile.package) { | ||
that.package.individually = serverlessFile.package.individually; | ||
that.package.path = serverlessFile.package.path; | ||
that.package.artifact = serverlessFile.package.artifact; | ||
that.package.exclude = serverlessFile.package.exclude; | ||
that.package.include = serverlessFile.package.include; | ||
that.package.excludeDevDependencies = serverlessFile.package.excludeDevDependencies; | ||
} | ||
|
||
return this; | ||
} | ||
|
||
setFunctionNames(rawOptions) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -109,6 +109,14 @@ class PluginInstall { | |
|
||
addPluginToServerlessFile() { | ||
return this.getServerlessFilePath().then(serverlessFilePath => { | ||
if (_.last(_.split(serverlessFilePath, '.')) === 'js') { | ||
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. Need the same process in 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. Good catch, added log message for uninstall as well 👍 |
||
this.serverless.cli.log(` | ||
Can't automatically add plugin into "serverless.js" file. | ||
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. Why cannot a plugin be installed automatically? 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. Because the js file only exports the serverless object, but hides the details how that is kept or generated. So the plugin plugin does not have any chance to tell the serverless.js to include the new plugin into the exported object the next time it is loaded. 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. OK, I see 😄 |
||
Please make it manually. | ||
`); | ||
return new BbPromise((resolve) => resolve()); | ||
} | ||
|
||
if (_.last(_.split(serverlessFilePath, '.')) === 'json') { | ||
return fse.readJsonAsync(serverlessFilePath).then(serverlessFileObj => { | ||
const newServerlessFileObj = serverlessFileObj; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -357,6 +357,24 @@ describe('PluginInstall', () => { | |
}); | ||
}); | ||
}); | ||
|
||
it('should not modify serverless .js file', () => { | ||
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. same here. |
||
const serverlessJsFilePath = path.join(servicePath, 'serverless.js'); | ||
const serverlessJson = { | ||
service: 'plugin-service', | ||
provider: 'aws', | ||
plugins: [], | ||
}; | ||
serverless.utils | ||
.writeFileSync(serverlessJsFilePath, `module.exports = ${JSON.stringify(serverlessJson)};`); | ||
pluginInstall.options.pluginName = 'serverless-plugin-1'; | ||
return expect(pluginInstall.addPluginToServerlessFile()).to.be.fulfilled.then(() => { | ||
// use require to load serverless.js | ||
// eslint-disable-next-line global-require | ||
expect(require(serverlessJsFilePath).plugins) | ||
.to.be.deep.equal([]); | ||
}); | ||
}); | ||
}); | ||
|
||
describe('#installPeerDependencies()', () => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,14 +22,20 @@ module.exports = { | |
const serverlessYmlFilePath = path.join(servicePath, 'serverless.yml'); | ||
const serverlessYamlFilePath = path.join(servicePath, 'serverless.yaml'); | ||
const serverlessJsonFilePath = path.join(servicePath, 'serverless.json'); | ||
const serverlessJsFilePath = path.join(servicePath, 'serverless.js'); | ||
|
||
return fileExists(serverlessYmlFilePath) | ||
.then(ymlExists => { | ||
if (!ymlExists) { | ||
return fileExists(serverlessYamlFilePath) | ||
.then(yamlExists => { | ||
if (!yamlExists) { | ||
return serverlessJsonFilePath; | ||
return fileExists(serverlessJsonFilePath).then((jsonExists) => { | ||
if (jsonExists) { | ||
return serverlessJsonFilePath; | ||
} | ||
return serverlessJsFilePath; | ||
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. Shouldn't we check here for the existence of the JS file too and reject with an error as last resort? return fileExists(serverlessJsFilePath).then(jsExists => {
if (jsExists) {
return serverlessJsFilePath;
}
return BbPromise.reject(new this.serverless.classes.Error('Could not find any serverless service definition file.'));
}); 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. Refactored this method a bit, looks much nicer now. |
||
}); | ||
} | ||
return serverlessYamlFilePath; | ||
}); | ||
|
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.
Do not use the
new Promise
anti-pattern! Additionally require can throw synchronously, so it has to be catched.