Skip to content

WIP Feature/concat schemas - #432

Open
dmitryt wants to merge 7 commits into
metarhia:masterfrom
dmitryt:feature/concat-schemas
Open

dmitryt wants to merge 7 commits into
metarhia:masterfrom
dmitryt:feature/concat-schemas

Conversation

@dmitryt

@dmitryt dmitryt commented Aug 3, 2022

Copy link
Copy Markdown
Contributor
  • tests and linter show no problems (npm t)
  • tests are added/updated for bug fixes and new features
  • code is properly formatted (npm run fmt)
  • description of changes is added in CHANGELOG.md
  • update .d.ts typings

Comment thread lib/schema.js
return null;
}

static concat(...sources) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try to use Schema.protoype.toJSON to reduce amount of code. You can create new Schema with just typings from toJSON

Comment thread lib/schema.js
// Q: How to merge options?
for (const schema of schemas) {
resultSchema.updateFromSchema(schema);
// Q: Can we have conflicts with indexes?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need examples of it, write tests please

Comment thread test/schema.js
const schema2 = Schema.from({ c: 'string', d: 'number' });
const schema3 = Schema.from({ e: 'string', f: 'number' });
test.strictEqual(
Schema.concat(schema1, schema2, schema3).toString(),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use toJSON to check typings

Comment thread test/schema.js
}
const schema3 = Schema.from({ Custom: {}, type: 'string' });
const schema4 = Schema.from({ Custom: {}, type: 'number' });
try {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use test.throws(() => fn(), new Error(your error message))

Comment thread test/schema.js
(test) => {
const schema1 = Schema.from({ a: 'string', b: 'number' });
const schema2 = Schema.from({ Custom: {}, type: 'string' });
try {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use test.throws

Comment thread lib/schema.js
}
}
const resultSchema = Schema.from(defs);
// Q: How to merge options?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need to merge options and concat validate functions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants