feat: bundle references into components #46
Conversation
|
@jonaslagoni can you review this once, I am able to bundle the references into the components. Bundling https://github.com/Souvikns/bundler/blob/fix-issue-34/example/asyncapi.yaml from https://github.com/Souvikns/bundler/blob/fix-issue-34/example/main.yaml and https://github.com/Souvikns/bundler/blob/fix-issue-34/example/messages.yaml |
There was a problem hiding this comment.
As I understand your implementation, you only bundle messages right? So schema payloads are never bundled into schemas component? Do you plan on doing this at a later stage? 🤔
Some general comments:
- Would be great to introduce a linter, as otherwise there can be quite the discrepancies between implementations 🙂 For example, it will catch those extra new lines and missing
;. - Remember to add tests (really good you have examples as well!) as they are to be executed on each PR/change, feel free to ping me if they cause any issues.
- Remember JSDoc function comments, rarely comments inside the function necessary, but it's always nice to have an explanation of what the function is intended to do 🙂
Otherwise it looks great with the messages being bundled 👍
|
As of now, we are only moving the messages, I think we can create an interface for adding rules for these operations and then reuse it to just add custom rules for Should I do this in this PR or in the next one? I also think this would be the right time to move to typescript |
Always split up the PRs into smaller ones, each with it's own feature when possible 🙂 It is much easier for the reviwers and for anyone else interrested. |
|
I have made all the changes you have suggested in this PR In the next PR, I will
|
|
I have added an option, so now to resolve external messages references into components the users can just pass in a flag otherwise it works as usual. I have also updated the readme, I think I am ready for review 🤞🏼. |
|
Need approval from @derberg to merge |
|
because users will most probably copypaste it. |
Shall I remove it, if people are going to copy/paste won't having the import help? |
No, it simply is a one more typo. - const fs = requrie('fs')
+ const fs = require('fs')I meant that users will most probably copypaste it, and copypaste it with the error it contains. Also, there are typos here: const bundle = requrie('@asyncapi/bundler');
const path = requrie('path');It might be a good idea to go through all PR proofreading it. |
|
/rtm |
|
🎉 This PR is included in version 0.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Since we are using
json-schema-ref-parserfor referencing external refs we could use it to reference all the external references and keep internal references intact, which results in documents having only internal references, but to bundle external references to correct properties we need to write our custom resolving logic. This is what PR aims to add, a way to update the external references to point to an internal object and successfully bring only the part of the object needed from the external file.Related issue(s)
Fixes #34