asyncapi / bundler

Combine multiple AsyncAPI specification files into one.
Apache License 2.0
29 stars 15 forks source link

fix: add interface `Options`, remove pre-validation with `Parser` #188

Closed aeworxet closed 1 month ago

aeworxet commented 2 months ago

This PR:

aeworxet commented 2 months ago

It appeared that Bundler doesn't always receive a valid AsyncAPI Document as an input. A non-valid AsyncAPI Document, which still remains to be a valid JSON/YAML, passes the bundling process because @apidevtools/json-schema-ref-parser doesn't care if a valid JSON is also a valid AsyncAPI Document. If there was an error in the initial AsyncAPI Document, it goes further, and as a result, the user ends up wondering what's wrong with their AsyncAPI Document if no errors were reported in between.

Users also don't always validate the outputted AsyncAPI Document because they used The Official App for bundling; how could it have bundled something improperly?

So, practice has shown that Bundler ought to satisfy the user's hope of getting a fully valid AsyncAPI Document without the need to use any other tool.

In fact, I would prefer to validate the data twice, both input and output, but validating the input wouldn't catch errors in the $refed AsyncAPI Documents. So, I want to at least validate the output, which will also catch the wrong input, if there was any.

And if I use the Parser anyway, then I can also delegate it watching over new formats (Spec versions) of AsyncAPI Documents, eliminating the need to update local types with each major change in the Spec.

Souvikns commented 2 months ago

If the user needs to validate the specifications, then the user can validate the spec using @asyncapi/parser before the bundling process. We can do this in CLI as well so users using the CLI will always get validated before bundle process starts. Anyone installing the bundler wants to use it as a library and so they can custom validate thier spec.

aeworxet commented 2 months ago

@derberg Should the approach described in https://github.com/asyncapi/bundler/issues/26 be reconsidered since two years have passed already, or should I remove the pre-validation with Parser and the @asyncapi/parser dependency altogether (pre-validation has proven useful by catching errors early, and also in Optimizer (I'll have to remove it from there too)?)

derberg commented 2 months ago

yeah, basically if parser is just used for validation and nothing else, can be removed and just readme should be clear about it, and that if someone needs validation, they should use parser. And as @Souvikns wrote, in CLI we can use existing code to enable validation before bundling

aeworxet commented 2 months ago

Pre-validation with Parser helped me catch a lot of early errors, but if you both insist. :shrug:

It was never documented; it simply was there for several versions, so I didn't change the README.md.

sonarcloud[bot] commented 2 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

derberg commented 2 months ago

prevalidation with parser is still recommended, it's just that it should be documented and suggested in readme. If someone wants to use bundler on their own, they should also use parser-js and in case of AsyncAPI CLI, bundling should be done with initial validation using parser that is there in CLI already

aeworxet commented 1 month ago

I'll address that in a separate README update then.

aeworxet commented 1 month ago

/rtm

asyncapi-bot commented 1 month ago

:tada: This PR is included in version 0.6.3 :tada:

The release is available on:

Your semantic-release bot :package::rocket: