Conversation
Removed @types/whatwg-fetch which caused "Duplicate identifier" errors (see http://stackoverflow.com/a/42429014).
| @@ -9,16 +9,14 @@ import { consts } from '../consts'; | |||
| describe('ValidationEngine Validate Form', () => { | |||
There was a problem hiding this comment.
I think mocha, chai and core-js imports are not necessary, isn't it?
There was a problem hiding this comment.
It would be better to do it in another issue.
|
|
||
| it('Spec #2 => should return a succeeded FormValidationResult with one fieldErrors equals { succeeded: true, key: "nameId", type: "REQUIRED", errorMessage: "" }' + | ||
| 'When passing one validation rule with key equals "nameId" and viewModel equals { id: "1", fullname: "john" }', (done) => { | ||
| it('Spec #2 => should return a succeeded FormValidationResult with one fieldErrors equals ' + |
There was a problem hiding this comment.
I think that we can use backticks for this kind of multiline spec description
There was a problem hiding this comment.
I was thinking about that... but remember that even if we use backticks messages may be displayed with same indentation they have in the text editor (at least we don't use 4-8 tabs wide)...
| formFieldToViewModelKeyValues.forEach((formFieldToViewModel: FormNameToFieldNameMapping) => { | ||
| const vmFieldValue = vm[formFieldToViewModel.vmFieldName]; | ||
| if (this.areParametersDefined(vm, validationFn)) { | ||
| for (let vmKey in vm) { |
There was a problem hiding this comment.
Other iteration could be using Object.keys(vm).forEach(...) but it's ok with that
There was a problem hiding this comment.
Yep, more beautiful, but more steps... (it was my first approach).
| "@types/react-redux": "^4.4.32", | ||
| "@types/redux": "^3.6.31", | ||
| "@types/redux-thunk": "^2.1.31", | ||
| "@types/whatwg-fetch": "0.0.32", |
There was a problem hiding this comment.
Why is this typings not necessary?
There was a problem hiding this comment.
New versions of TypeScript already have fetch typings. It was causing duplicating export conflicts, see this
There was a problem hiding this comment.
Finally we have to uninstall whatwg-fetch lib?
There was a problem hiding this comment.
No we don't. The polyfill is still needed but typings.
|
@JaimeSalas and my self reviewed, pending on @nasdan feedback, once you are ok with him please proceed with the merge. Thanks. |
|
Finally, what version should be used for this update?? |
3 participants
This closes #37.