graphile / crystal

🔮 Graphile's Crystal Monorepo; home to Grafast, PostGraphile, pg-introspection, pg-sql2 and much more!
https://graphile.org/
Other
12.49k stars 568 forks source link

Ruru rendering issues #2103

Open FelixZY opened 1 week ago

FelixZY commented 1 week ago

Summary

Not sure how to put this down in a good way (sorry for multi-bug). Basically, Ruru does not behave as I would expect.

Screencast from 2024-06-21 21:07:52.webm

Some issues:

Steps to reproduce

This is the comment I was using:

comment on column labelt_public.images.data_uri is $$
  @notNull

  A [data URI](https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URLs)
  with base64 encoded data.

  The URI must use an image MIME type and base64 encoded data.
  A simple way to verify a given URI is to use this
  regex: `^data:image/\w+;base64,[A-z0-9+/]+={0,2}$`.
$$;

Expected results

The output in ruru should not be flaky and it should adhere to proper markdown (where single newlines are ignored)

Actual results

See above

Additional context

postgraphile: 5.0.0-beta.26 ruru: 2.0.0-beta.13 Google Chrome: Version 125.0.6422.141 (Official Build) (64-bit)

benjie commented 1 week ago

Are these Ruru specific issues, or are they issues in the upstream project GraphiQL? If the latter, please file there.

It’s also common to use GitHub-flavoured markdown, which like this block does indeed include line feeds when entered, without requiring the markdown-standard double space.

Since GraphiQL is the reference implementation of a GraphQL IDE, I defer to its choices, even if I don’t agree with them.

FelixZY commented 1 week ago

Are these Ruru specific issues, or are they issues in the upstream project GraphiQL?

I don't know. I'd guess like 50/50? There's an obvious problem with the Postgraphile SQL parser given the flaky behaviour in the video. However, it's likely that the "hidden second paragraph" issue is graphiql upstream (though I never had issues with this until I started trying to record reproduction videos so that seems flaky as well 🤔. Closest I could find for now was https://github.com/graphql/graphiql/issues/3348).

It’s also common to use GitHub-flavoured markdown, which like this block does indeed include line feeds when entered, without requiring the markdown-standard double space.

I was not aware of this "feature" (bug imo). According to stack overflow, this is no longer a documented feature:

Interpreting newlines as <br /> used to be a feature of Github-flavored markdown, but the most recent help document no longer lists this feature.

I guess it makes sense for Postgraphile's parser to output the comment as-written and rely on the browser to render it nicely - as long as Postgraphile's parser does not itself add <br/>s I guess I'm happy here. I'll see if I can validate that somehow.

I found a PR in upstream to fix this: https://github.com/graphql/graphiql/pull/3414