Chrimc/form #394
Chrimc/form #394
Conversation
template/prop loops.
shortcut to debug
Generate multilingual .lg file.
Nigao/help
Support multiple templates. Add *PROPN** pattern.
Change unionType to union. Add documentation.
There was a problem hiding this comment.
👍 VS Code stuff looks good
left some minor comments and nits. one thing we should consider long term is moving some of the inline string literals into a collection of constants. If we need to refactor in the future, it can be nice to have use some common constants instead of literals we're grepping through the code to find/fix.
| @@ -313,14 +313,14 @@ export default class DialogMerge extends Command { | |||
| } | |||
|
|
|||
| processRoles(definitions: any, metaSchema: any): void { | |||
| for (let type in definitions) { | |||
| this.walkJSON(definitions[type], (val: any, _obj, key) => { | |||
| for (let kind in definitions) { | |||
There was a problem hiding this comment.
[nit] For for in statements, it is best practice to use a hasOwnProperty filter.
for (const prop in obj) {
if (obj.hasOwnProperty(prop)) {
console.log(`obj.${prop} = ${obj[prop]}`);
}
}https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...in
I personally prefer for of. In this case I'd recommend:
for (const kind of Object.keys(definitions)) {I'll be adding some tslint rules to enforce this, so you can ignore for now if you'd like.
There was a problem hiding this comment.
Not sure I buy this. We know the is a plain old JSON object, not a class.
| @@ -329,50 +329,50 @@ export default class DialogMerge extends Command { | |||
| } | |||
| } | |||
|
|
|||
| processRole(role: string, elt: any, type: string, definitions: any, metaSchema: any, key?: string): void { | |||
| const prefix = 'unionType(' | |||
| processRole(role: string, elt: any, kind: string, definitions: any, metaSchema: any, key?: string): void { | |||
There was a problem hiding this comment.
[nit] you should avoid any types if you do have an expected type
There was a problem hiding this comment.
The linting rules I have generally complain if you do not--maybe when you add consistent ones I won't see that. I also think that if you are buying into typescript it is not unreasonable to be explicit.
Integrate $kind change.
Add $public.
Add lg expansion to schema.