-
-
Notifications
You must be signed in to change notification settings - Fork 132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fetching schema by url in library #34
Changes from 30 commits
46b9ba6
25731f5
997a4e3
5cce961
9a1fa9d
e376cc4
6c53350
2f6321a
07e637d
217d828
159edd8
45f63cb
5de9286
6ed172b
28249d7
fea738c
ed13f0b
e94b478
bf968c2
c6995ef
a8ee45f
1e8f736
c90c365
7e6627d
ad31428
3aa7465
6ba43f7
6da198c
ecb381f
a4b3f1d
d09c1f5
f9ac779
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,13 @@ import { ThemeProvider } from 'styled-components'; | |
import { AsyncApi, SecurityScheme } from '../../types'; | ||
import { ThemeInterface, defaultTheme } from '../../theme'; | ||
import { ConfigInterface, defaultConfig } from '../../config'; | ||
import { parser, beautifier } from '../../helpers'; | ||
import { | ||
parser, | ||
beautifier, | ||
FetchingSchemaInterface, | ||
fetchSchema, | ||
stringify, | ||
} from '../../helpers'; | ||
|
||
import InfoComponent from '../Info/Info'; | ||
import Security from '../Security/Security'; | ||
|
@@ -16,7 +22,8 @@ import ErrorComponent from '../Error/Error'; | |
import { AsyncApiWrapper } from './styled'; | ||
|
||
export interface AsyncApiProps { | ||
schema: string | Object; | ||
schema?: string | Object; | ||
urlSchema?: FetchingSchemaInterface; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we enforce that either schema or urlSchema must be provided? I know it's possible with prop-types, but not sure about Typescript. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is possible by use type There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I implemented my proposition from second comment in this thread. Please see: https://github.com/asyncapi/asyncapi-react/pull/34/files#diff-2fa873daa631d9246738398633704fd9R25 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! I love this solution. Just one question: I see it says There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Understood. Thanks! :) |
||
theme?: Partial<ThemeInterface>; | ||
config?: Partial<ConfigInterface>; | ||
} | ||
|
@@ -53,12 +60,28 @@ class AsyncApiComponent extends Component<AsyncApiProps, AsyncApiState> { | |
} | ||
|
||
async componentWillMount() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Heads up There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
this.prepareSchema(this.props.schema); | ||
const { schema, urlSchema } = this.props; | ||
|
||
if (schema) { | ||
await this.prepareSchema(schema); | ||
} else if (urlSchema) { | ||
await this.prepareSchema(await fetchSchema(urlSchema)); | ||
} | ||
} | ||
|
||
async componentWillReceiveProps(nextProps: AsyncApiProps) { | ||
if (nextProps.schema !== this.props.schema) { | ||
this.prepareSchema(nextProps.schema); | ||
const { schema, urlSchema } = nextProps; | ||
|
||
if (schema && schema !== this.props.schema) { | ||
await this.prepareSchema(schema); | ||
} else if (urlSchema) { | ||
const stringifiedUrlSchema = stringify(urlSchema); | ||
if ( | ||
stringifiedUrlSchema && | ||
stringifiedUrlSchema !== stringify(this.props.urlSchema) | ||
) { | ||
await this.prepareSchema(await fetchSchema(urlSchema)); | ||
} | ||
} | ||
} | ||
|
||
|
@@ -98,6 +121,8 @@ class AsyncApiComponent extends Component<AsyncApiProps, AsyncApiState> { | |
|
||
if (!(validatedSchema && validated)) return null; | ||
|
||
console.log(validatedSchema); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was it forgotten here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hah :D I always forget about console.logs in my code. Thanks! |
||
|
||
return ( | ||
<ThemeProvider theme={concatenatedTheme}> | ||
<AsyncApiWrapper> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
export interface FetchingSchemaInterface { | ||
url: string; | ||
requestOptions?: RequestInit; | ||
} | ||
|
||
const defaultRequestOptions: RequestInit = { | ||
method: 'GET', | ||
}; | ||
|
||
export const fetchSchema = async ({ | ||
url, | ||
requestOptions = defaultRequestOptions, | ||
}: FetchingSchemaInterface): Promise<any> => { | ||
return fetch(url, requestOptions).then(handleResponse); | ||
}; | ||
|
||
function handleResponse(response: any) { | ||
return response.text().then((data: string) => data); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,4 @@ | ||
export * from './parser'; | ||
export * from './beautifier'; | ||
export * from './beautifier'; | ||
export * from './fetchSchema'; | ||
export * from './json-parser'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
export const parse = <T extends {}>(str?: string): T => { | ||
if (!str) return {} as T; | ||
|
||
try { | ||
return JSON.parse(str) as T; | ||
} catch (e) { | ||
return {} as T; | ||
} | ||
}; | ||
|
||
export const stringify = <T extends {}>(content?: T): string => { | ||
if (!content) ''; | ||
|
||
try { | ||
return JSON.stringify(content); | ||
} catch (e) { | ||
return ''; | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about rewriting this copy?
Either
schema
orurlSchema
prop must be present.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kazydek must check this, but thank you for your proposition :)