Make processed input path absolute #156
Conversation
When using the invoke local command with the --path (-p) flag, relative file paths will not be found because serverless will end up looking in the build directory later on. So, this commit ensures that the path is absolute, ensuring it will be found, even if the path was relative. Couldn't think of a good name for the function. Also, I'm not sure that this should go in the utils file. Happy to make revisions!
|
I'm thinking that |
|
Yes, agree. |
There was a problem hiding this comment.
As soon as you moved it to its own file, you also should add unit tests for the feature that check the resulting output of the function with various inputs.
Just thought about unit tests. I will take care of them - as this PR is only a correction of the main PR :)
lib/utils.js
Outdated
| @@ -32,8 +35,18 @@ function searchCache(moduleName, callback) { | |||
| } | |||
| } | |||
|
|
|||
| function makeProcessedInputPathAbsolutePath() { | |||
| var originalPath = this.serverless.config.serverless.processedInput.options.path | |||
There was a problem hiding this comment.
Should be const instead of var. Also a missing semicolon.
lib/utils.js
Outdated
| function makeProcessedInputPathAbsolutePath() { | ||
| var originalPath = this.serverless.config.serverless.processedInput.options.path | ||
| if (originalPath) { | ||
| var absolutePath = path.resolve(originalPath) |
There was a problem hiding this comment.
const and semicolon.
Did you check this for a given absolute path (i.e. if originalPath is already absolute) too?
There was a problem hiding this comment.
Did you check this for a given absolute path (i.e. if originalPath is already absolute) too?
Yes, this works with absolute paths as well.
> path.resolve('foo')
'/Users/Ben/js_projects/serverless-webpack/foo'
> path.resolve('./foo')
'/Users/Ben/js_projects/serverless-webpack/foo'
> path.resolve('/Users/Ben/js_projects/serverless-webpack/foo')
'/Users/Ben/js_projects/serverless-webpack/foo'
|
@HyperBrain thanks, I've made some revisions:
Let me know if there is anything else I can do! |
|
@benjaminwood It's good to go now 🙌 . I'll merge it into the feature branch and continue with the finalization. Then we have a PR for the complete feature onto master and can do a final test and review. I'll let you know if there is anything you can jump in - just thought about something 😄 : If you don't mind, you could dive a bit into the source-map stuff mentioned in #108 and check how sourcemaps can be used properly with local invoke. If they work, we should create a PR to update the documentation (README) and show users how to setup a working system. |
|
Thanks @HyperBrain! I'll take a look at #108. |
* Make processed input path absolute When using the invoke local command with the --path (-p) flag, relative file paths will not be found because serverless will end up looking in the build directory later on. So, this commit ensures that the path is absolute, ensuring it will be found, even if the path was relative. Couldn't think of a good name for the function. Also, I'm not sure that this should go in the utils file. Happy to make revisions! * Move execution building out of utils * Add lib/makePathOptionAbsolute.js
* Make processed input path absolute When using the invoke local command with the --path (-p) flag, relative file paths will not be found because serverless will end up looking in the build directory later on. So, this commit ensures that the path is absolute, ensuring it will be found, even if the path was relative. Couldn't think of a good name for the function. Also, I'm not sure that this should go in the utils file. Happy to make revisions! * Move execution building out of utils * Add lib/makePathOptionAbsolute.js
* Make processed input path absolute When using the invoke local command with the --path (-p) flag, relative file paths will not be found because serverless will end up looking in the build directory later on. So, this commit ensures that the path is absolute, ensuring it will be found, even if the path was relative. Couldn't think of a good name for the function. Also, I'm not sure that this should go in the utils file. Happy to make revisions! * Move execution building out of utils * Add lib/makePathOptionAbsolute.js
* Make processed input path absolute When using the invoke local command with the --path (-p) flag, relative file paths will not be found because serverless will end up looking in the build directory later on. So, this commit ensures that the path is absolute, ensuring it will be found, even if the path was relative. Couldn't think of a good name for the function. Also, I'm not sure that this should go in the utils file. Happy to make revisions! * Move execution building out of utils * Add lib/makePathOptionAbsolute.js
* Make processed input path absolute When using the invoke local command with the --path (-p) flag, relative file paths will not be found because serverless will end up looking in the build directory later on. So, this commit ensures that the path is absolute, ensuring it will be found, even if the path was relative. Couldn't think of a good name for the function. Also, I'm not sure that this should go in the utils file. Happy to make revisions! * Move execution building out of utils * Add lib/makePathOptionAbsolute.js
What did you implement:
Related: #151
When using the invoke local command with the --path (-p) flag, relative file paths will not be found because serverless will end up looking in the build directory later on. So, this PR ensures that the path is absolute, ensuring it will be found, even if the path was relative.
How did you implement it:
I added a function in utils.js that gets the --path option if present. It then uses
path.resolveto resolve it, then assigns it back to serverless config.How can we verify it:
Just use
invoke localwith the --path option:serverless invoke local -f function_name --path data.jsonTodos:
Is this ready for review?: YES
Is it a breaking change?: NO