chore: merge release into master #4685
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…con-rgba fix: generated App Store icon has alpha channel
Currently when you run `tns cloud run ...` the command executes local prepare without taking into account the value of `useLegacyWorkflow` value in nsconfig.json file. The problem is that cloud commands have specific `--` options, which are set as `dashedOptions` on command level. During command execution the following actions happen: 1. CLI starts its execution and via `$injector` constructs new instance of `$options`. 2. In the constructor of `$options` we call `this.setArgv()`, which calls `yargs` package and constructs values based on the user input of `--` options. 3. `$injector` resolves `$commandsService` and constructs new object from it. 4. In the constructor of `CommandsService`, we call `$options.setupOptions` and pass the current projectData. `setupOptions` takes into account the projectData and again calls `this.setArgv()`. After that it overwrites some of the values set by yargs based on the projectData. 5. `CommandsService` starts execution of the command by resolving new instance of it and calling its own `tryExecuteCommandAction` method. 6. `tryExecuteCommandAction` method internally calls `this.$options.validateOptions` by passing the command specific options(`dashedOptions`). The `validateOptions` method modifies the internal structure based on which yargs constructs the passed options and calls `this.setArgv()` again. At this point we overwrite the internal values of `$options` based on the user's input and taking into account the command's `dashedOptions`. However, this way we've overwritten the values set by `setupOptions` in step 4 which are based on the current project dir. To fix the behavior, make `setupOptions` private in `$options` and call it internally in the `validateOptions` method. So the new workflow is: 1. CLI starts its execution and via `$injector` constructs new instance of `$options`. 2. In the constructor of `$options` we call `this.setArgv()`, which calls `yargs` package and constructs values based on the user input of `--` options. 3. `$injector` resolves `$commandsService` and constructs new object from it. 4. `CommandsService` starts execution of the command by resolving new instance of it and calling its own `tryExecuteCommandAction` method. 5. `tryExecuteCommandAction` method internally calls `this.$options.validateOptions` by passing the command specific options(`dashedOptions`). The `validateOptions` method calls `this.setupOptions` and pass the current projectData and command specific options. After that it calls `this.setArgv()` and the internal structure that contains the passed options is overwritten based on the command specific options. After that the method overwrites some of the values based on the passed projectData.
In case you are using unsupported Node.js version CLI must show you error that this version is not supported and to inform you which is the supported range. Currently this is not working and CLI shows error for `Unexpected token`. The problem is that CLI has some specific logic to check the Node.js version and the code should be pure ES5 syntax in order to work with old Node.js versions (by default CLI's code is ES6). The current issue is caused by using string interpolation in the mentioned code. Fix the message to fix the problem.
fix: cloud run command does not respect useLegacyWorkflow flag
Several logger methods have been deprecated, but their usage had been broken. Fix it until we delete them. The current problem arises when we pass several args to the `logger.out` (or any deprecated method).
…logger-apis fix: pass correct args when using deprecated logger API
…error fix: incorrect error for non-supported Node.js version
In case you try printing the following:
```
const obj1 = null;
this.$logger.info("value %s", obj1);
```
Our implementation fails with error: `Cannot read property hasOwnProperty of null`.
The problem is in the way we check for passed logger options in the args passed to `logger` methods. The `typeof null` returns object, so next parts of the code fail.
Also passed logger options are handled by priority, i.e. the first passed option should be used. This allows callers of methods that specify some logger options internaly, to overwrite their values. Add unit tests for this behavior.
Fix the case when same logger option is passed multiple times in different objects - currently only one of the instances is removed and the other one is included in the message visible to the user. Now all such objects are removed.
Add unit tests for all of the described cases.
fix: logger fails to print null objects
DimitarTachev
approved these changes
Jun 6, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Checklist
What is the current behavior?
What is the new behavior?
Fixes/Implements/Closes #[Issue Number].