openapi-format icon indicating copy to clipboard operation
openapi-format copied to clipboard

yaml document start characters `---` & comments not preserved

Open skarzi opened this issue 3 years ago • 12 comments

Hi :wave:

First of all thanks for developing this awesome lib!

I am using the openapi-format more like a validator (mainly because it removes comments like mentioned in https://github.com/thim81/openapi-format/issues/44), not as a formatted, and in some projects, we are using --- (a YAML document start sign - as described in 2.2 Structure), but running the openapi-format on such documents produces output without --- and it would be nice to preserve --- when it's present or add some option to always add/remove it from the file.

PS. https://github.com/thim81/asyncapi-format also does not preserve ---

skarzi avatar Jun 21 '22 15:06 skarzi

@skarzi Thanks for taking the time to report the issue.

Could you share a small sample of how you use the --- in the OpenAPI document?

That way I better understand the issue and I can create a fix/test for the unwanted behaviour.

For your info, openapi-format is using @stoplight/yaml as YAML parser, so it might be possible that the parser removes the ---.

(asyncapi-format is a bit behind openapi-format, but I do try to keep the feature set in sync as much as possible)

thim81 avatar Jun 21 '22 16:06 thim81

I am just adding --- at the beginning of the file, e.g.

---
openapi: 3.0.0
info:
  version: 1.0.0
  description: Empty API description
  title: Empty API description
  license:
    name: MIT
  contact:
    email: [email protected]
servers:
  - url: http://test.com/fantastic-api
paths: {}

and openapi-format simply strips --- from the API description.

But you are right that this issue is related to @stoplight/yaml (and precisely @stoplight/yaml-ast-parser where safeDump is defined).

skarzi avatar Jun 21 '22 19:06 skarzi

@skarzi Would you have some time to play around with some @stoplight/yaml safeDump options? To see if there is a setting that would keep the ---?

I look at the YAML spec but it is not clear what the --- does?

thim81 avatar Jun 22 '22 11:06 thim81

I have checked and it's impossible to prepend --- to the document with @stoplight/yaml :(

However, I have found another YAML lib - https://github.com/eemeli/yaml - that supports document start characters as well as comments. Are there any special reasons you have chosen @stoplight/yaml lib to work with the YAML content? Because maybe we could try to migrate to eemli/yaml, so comments and --- will be supported. Another positive aspect about this lib is that it uses (and passes all) https://github.com/yaml/yaml-test-suite.

skarzi avatar Jun 22 '22 19:06 skarzi

@skarzi The main reason why we implemented @stopligth/yaml

https://github.com/thim81/openapi-format#stoplight-studio

We have adopted the YAML parsing style from Stoplight Studio, by leveraging the @stoplight/yaml package for handling the parsing of OpenAPI YAML files.

By using the Stoplight YAML parsing, the results will be slightly different from when using a normal YAML parsing library, like js-to-yaml. We appreciate the Stoplight Studio tool, since it is an excellent GUI for working with OpenAPI documents for non-OpenAPI experts who will be contributing changes. By adopting the Stoplight Studio YAML parsing, the potential risk of merge conflicts will be lowered, which is the main reason why we opted for using the @stoplight/yaml package.

thim81 avatar Jun 23 '22 06:06 thim81

By using the Stoplight YAML parsing, the results will be slightly different from when using a normal YAML parsing library

What exactly are the differences?

By adopting the Stoplight Studio YAML parsing, the potential risk of merge conflicts will be lowered

Could you clarify this in more detail?

skarzi avatar Jun 23 '22 11:06 skarzi

The biggest difference was, the way multiline text was handled (| >) and quoting properties to make sure they are valid

The merge conflicts would be. If you open an OpenAPI file in Stoplight studio, it automatically parses the YAML file using the @stoplight/yaml package. So if you would use Stoplight Studio in combination with openapi-format, there were always changes to items were not touched (because of the parsing differences). This could lead to merge conflicts and human-error because it might not be clear anymore what was now actually changes.

thim81 avatar Jun 23 '22 16:06 thim81

Thanks for providing more details!

I will experiment with https://github.com/eemeli/yaml and give you feedback so we can decide what to do next.

My new idea is to allow users to choose the YAML parsing engine/lib via some CLI flag, but before it is nice to test eemeli/yaml.

skarzi avatar Jun 27 '22 09:06 skarzi

I have checked https://github.com/eemeli/yaml it seems to work pretty differently than https://github.com/stoplightio/yaml, the most significant differences I have spotted are:

  • ' and " are used differently (I don't understand the logic of stoplight/yaml, but eemeli/yaml uses either " or ' whenever it's required)
  • multiline strings are handled differently and it will be hard to adjust eemeli/yaml to output multiline strings identically to stoplight/yaml

Appending --- to YAML with eemli/yaml is pretty straightforward, it's enough to add directives: true to stringify options. But dumping comments together with the entire content is complicated because it requires using the Document class to iterate and manipulate the YAML content.

skarzi avatar Jul 01 '22 12:07 skarzi

@skarzi Thanks for taking the time to do the investigation and reporting your findings.

Looking at the outcome, what is your conclusion on the Stoplight YAML vs Eemeli YAML? It feels like the benefits of switching are rather low, so my feeling leans towards keeping as-is? But I'm curious about your opinion.

thim81 avatar Jul 26 '22 09:07 thim81

API descriptions are source code (they are parsed by some tools e.g. to generate HTML docs or run the mock servers as well as read/edited by humans), so I think that features like preserving comments are fundamental because in most cases humans are working with the API descriptions directly (in it raw YAML form), however, the implementation cost is pretty high because we probably need to implement some proxy object that will translate all JS Object operation into proper Document ones, so in the end, keeping as-is i is probably the better option, but it will be nice to get feedback also from other people ;)

skarzi avatar Aug 01 '22 16:08 skarzi

Hi @skarzi

I agree with your comment. It would be expected behaviour to keep comments, but like your findings I ran into the result that YAML parsing and keeping comments are not so straightforward. openapi-format has as goal to format the OpenAPI document, so in that sense removing "internal comments" is a positive side-effect. It would be nice to support the keeping of comments & --- in the future, so lets keep this issue open. I'll modify the title to broaden the scope of the item a bit.

If anybody has found a straightforward YAML parser, that can preserve comments & ---, feel free to share your input in the item.

thim81 avatar Aug 07 '22 13:08 thim81

Update: Stoplight YAML-ast-parser introduced the option to preserve comments on YAML docs, https://github.com/stoplightio/yaml-ast-parser/pull/6

Will need some more investigation

thim81 avatar Aug 17 '24 10:08 thim81

@skarzi I'm working on a #136 to introduce --keepComments which keep the YAML comments in a file.

I need some more tests to see how it behaves in combination of sorting & filtering.

thim81 avatar Sep 08 '24 14:09 thim81

We just release v1.23.0 which provides the option --keepComments which keep the YAML comments in a file.

Closing this issue. If you're still experiencing problems, please feel free to reopen this issue or create a new one. Thanks your input and patience!

thim81 avatar Sep 10 '24 16:09 thim81