Fetching schema by url in library #34

magicmatatjahu · 2019-01-31T10:18:15Z

Description

Changes proposed in this pull request:

Add feature to fetch schema from external resources in library package

Related issue(s)
Resolves #32

…lerna

… spec before validation it

…eact

fmvilas

This is great stuff. I just left some comments. Keep up the great work 🙌

fmvilas · 2019-01-31T10:29:50Z

README.md

@@ -45,6 +49,8 @@ The list of props for the AsyncApi React component includes:

     > **NOTE:** The `Partial<T>` type means that every field in the `T` type is optional.

+   > **NOTE:** You must pass `schema` or `urlSchema` to component for correct work of component.


What about rewriting this copy?

Either schema or urlSchema prop must be present.

@kazydek must check this, but thank you for your proposition :)

fmvilas · 2019-01-31T10:31:49Z

library/src/containers/AsyncApi/AsyncApi.tsx

@@ -98,6 +121,8 @@ class AsyncApiComponent extends Component<AsyncApiProps, AsyncApiState> {

    if (!(validatedSchema && validated)) return null;

+    console.log(validatedSchema);


Was it forgotten here?

Hah :D I always forget about console.logs in my code. Thanks!

fmvilas · 2019-01-31T10:34:16Z

library/src/containers/AsyncApi/AsyncApi.tsx

@@ -53,12 +60,28 @@ class AsyncApiComponent extends Component<AsyncApiProps, AsyncApiState> {
  }

  async componentWillMount() {


Heads up componentWillMount is deprecated. Check out: https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.

componentWillMount and other will be deprecated on >= 17.0 versions. We use at the moment 16.6.3 version of React and we don't see any warnings related to the use of these lifecycles, but I will think whether to change it now or in the next PR.

fmvilas · 2019-01-31T10:38:27Z

library/src/containers/AsyncApi/AsyncApi.tsx

@@ -16,7 +22,8 @@ import ErrorComponent from '../Error/Error';
 import { AsyncApiWrapper } from './styled';

 export interface AsyncApiProps {
-  schema: string | Object;
+  schema?: string | Object;
+  urlSchema?: FetchingSchemaInterface;


Can we enforce that either schema or urlSchema must be provided? I know it's possible with prop-types, but not sure about Typescript.

Yes, this is possible by use type schema: string | Object | FetchingSchemaInterface. If developer pass FetchingSchemaInterface then we will fetch schema from url, otherwise we use schema :)

I didn't explain myself correctly. I mean having both schema and urlSchema, like it's right now, but enforcing in the code the rule of having just one of them.

I implemented my proposition from second comment in this thread. Please see: https://github.com/asyncapi/asyncapi-react/pull/34/files#diff-2fa873daa631d9246738398633704fd9R25

Nice! I love this solution. Just one question: I see it says string | Object | FetchSchemaInterface, will it always fallback into Object? Saying because an object of the form FetchSchemaInterface is also an object, right? Or will typescript take care of this?

Thanks :) You can pass schema as string (json or yaml), or Object (this is reference to AsyncAPIInterface) or FetchSchemaInterface. I agree with you that FetchSchemaInterface is also a Object, but Object with appropriate fields, so if dev pass Object as FetchSchemaInterface to component then component will execute whole code as FetchSchemaInterface, if pass as Object, then code validate this Object as AsyncApi and show errors if this spec is invalid :) You have to look at it in terms of static typing, not dynamic.

Understood. Thanks! :)

pkosiec

LGTM

kazydek · 2019-02-07T11:47:59Z

README.md


-     The `schema` property is required and contains AsyncAPI specification. It should be one of the `string` or [`AsyncApiInterface`](./library/src/types.ts#L13) type. For more information on what it contains and what it should look like, read [AsyncAPI Specification](https://github.com/asyncapi/asyncapi#asyncapi-specification).
+     The `schema` property is required and contains AsyncAPI specification. It should be one of the `string` or [`AsyncApiInterface`](./library/src/types.ts#L13) type or [`FetchingSchemaInterface`](./library/src/helpers/fetchSchema.ts#L1) object for fetching schema from external resource. For more information on what it contains and what it should look like, read [AsyncAPI Specification](https://github.com/asyncapi/asyncapi#asyncapi-specification).


Suggested change

The `schema` property is required and contains AsyncAPI specification. It should be one of the `string` or [`AsyncApiInterface`](./library/src/types.ts#L13) type or [`FetchingSchemaInterface`](./library/src/helpers/fetchSchema.ts#L1) object for fetching schema from external resource. For more information on what it contains and what it should look like, read [AsyncAPI Specification](https://github.com/asyncapi/asyncapi#asyncapi-specification).

The `schema` property is required and contains AsyncAPI specification. Use the `string` type, the [`AsyncApiInterface`](./library/src/types.ts#L13) type, or the [`FetchingSchemaInterface`](./library/src/helpers/fetchSchema.ts#L1) object to fetch the schema from an external resource. For more information on what it contains and what it should look like, read [AsyncAPI Specification](https://github.com/asyncapi/asyncapi#asyncapi-specification).

magicmatatjahu and others added 29 commits November 8, 2018 12:49

Add basic content: types, basic components and initial architecture

46b9ba6

Add Schema type and improve basic components

25731f5

Add parser (validator and dereferencer for json and yaml)

997a4e3

Add components: Messages, Schemas and Topics

5cce961

Add Message component and improve Messages components

9a1fa9d

Big commit

e376cc4

Improve components for better passing custom theme.

6c53350

Improve kyma theme

2f6321a

Audit packages

07e637d

Add highlights in schemaExample component

217d828

After review

159edd8

Update package.json: add prettier and basic info of reposirtory

45f63cb

Add tslint and prettier config file

5de9286

Add Apache 2.0 license

6ed172b

Setup tests configuration

28249d7

Split projects to two separate: playground and library and configure …

fea738c

…lerna

Improve Playground app

ed13f0b

Add new mock and add function to remove null or undefined values from…

e94b478

… spec before validation it

Add new License

bf968c2

Merge branch 'master' of https://github.com/kyma-incubator/asyncapi-r…

c6995ef

…eact

Merge branch 'master' of https://github.com/kyma-incubator/asyncapi-r…

a8ee45f

…eact

Merge branch 'master' of https://github.com/kyma-incubator/asyncapi-r…

1e8f736

…eact

Merge branch 'master' of github.com:asyncapi/asyncapi-react

c90c365

Merge branch 'master' of github.com:asyncapi/asyncapi-react

7e6627d

Merge branch 'master' of github.com:asyncapi/asyncapi-react

ad31428

Merge branch 'master' of github.com:asyncapi/asyncapi-react

3aa7465

Merge branch 'master' of github.com:asyncapi/asyncapi-react

6ba43f7

Merge branch 'master' of github.com:asyncapi/asyncapi-react

6da198c

Add feature to fetch schema from external resources in library package

ecb381f

magicmatatjahu added the enhancement New feature or request label Jan 31, 2019

magicmatatjahu added area/library Related to all activities around Library package area/documentation Related to all activities around documentation labels Jan 31, 2019

Update readme.md for new prop

a4b3f1d

magicmatatjahu requested a review from kazydek as a code owner January 31, 2019 10:23

fmvilas reviewed Jan 31, 2019

View reviewed changes

After review

d09c1f5

pkosiec approved these changes Feb 6, 2019

View reviewed changes

derberg approved these changes Feb 6, 2019

View reviewed changes

kazydek reviewed Feb 7, 2019

View reviewed changes

After Readme.md review

f9ac779

kazydek approved these changes Feb 7, 2019

View reviewed changes

magicmatatjahu merged commit f625e3a into asyncapi:master Feb 7, 2019

magicmatatjahu deleted the fetching-schema-by-url branch February 7, 2019 12:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fetching schema by url in library #34

Fetching schema by url in library #34

magicmatatjahu commented Jan 31, 2019

fmvilas left a comment

fmvilas Jan 31, 2019

magicmatatjahu Jan 31, 2019

fmvilas Jan 31, 2019

magicmatatjahu Jan 31, 2019 •

edited

Loading

fmvilas Jan 31, 2019

magicmatatjahu Jan 31, 2019

fmvilas Jan 31, 2019

magicmatatjahu Jan 31, 2019

fmvilas Jan 31, 2019

magicmatatjahu Jan 31, 2019

fmvilas Jan 31, 2019

magicmatatjahu Jan 31, 2019

fmvilas Jan 31, 2019

pkosiec left a comment

kazydek Feb 7, 2019

		@@ -45,6 +49,8 @@ The list of props for the AsyncApi React component includes:

		> NOTE: The `Partial<T>` type means that every field in the `T` type is optional.

		> NOTE: You must pass `schema` or `urlSchema` to component for correct work of component.

		@@ -98,6 +121,8 @@ class AsyncApiComponent extends Component<AsyncApiProps, AsyncApiState> {

		if (!(validatedSchema && validated)) return null;

		console.log(validatedSchema);

		@@ -53,12 +60,28 @@ class AsyncApiComponent extends Component<AsyncApiProps, AsyncApiState> {
		}

		async componentWillMount() {


		The `schema` property is required and contains AsyncAPI specification. It should be one of the `string` or [`AsyncApiInterface`](./library/src/types.ts#L13) type. For more information on what it contains and what it should look like, read [AsyncAPI Specification](https://github.com/asyncapi/asyncapi#asyncapi-specification).
		The `schema` property is required and contains AsyncAPI specification. It should be one of the `string` or [`AsyncApiInterface`](./library/src/types.ts#L13) type or [`FetchingSchemaInterface`](./library/src/helpers/fetchSchema.ts#L1) object for fetching schema from external resource. For more information on what it contains and what it should look like, read [AsyncAPI Specification](https://github.com/asyncapi/asyncapi#asyncapi-specification).

Fetching schema by url in library #34

Fetching schema by url in library #34

Conversation

magicmatatjahu commented Jan 31, 2019

fmvilas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magicmatatjahu Jan 31, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pkosiec left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magicmatatjahu Jan 31, 2019 •

edited

Loading