Scalar OpenAPI Docs Integration

[marclave]

hey 👋 platformatic,

I'm excited to get everyones feedback on integrating the Scalar Fastify plugin in Platformatic. I attached a preview of it working in the docs (see video below 👇 ).

Some features Scalar brings:

• Fully integrated REST API Client for testing endpoints (see video)
• Offline search powered by Fuse.js
• Easily customizable for on brand docs (we have the platformatic theme on default 💅 )
• Code Snippet generation in dozens of languages

An insane feature that was great to use when building out my demo project was to test the API right in the docs 🤯 felt like the developer experience for iterating and building was so fast! You can see it in the video :)

Some things we need to improve on but are a top priority 🖖

• we should add GZIP to serving the docs in platformatic
• Swagger-UI bundle size is 2MB vs the 2.2MB for Scalar (we can always get smaller)

I'm really excited to integrate Scalar, I think this is the beginning of a great partnership & I am eager for everyone's feedback on improving this PR & ensuring alignment before we get it merged ✨

CleanShot.2023-11-29.at.10.59.30.mp4

edit
I can squash my commits after feedback :)

[mikaelkaron]

I wonder if rather than shifting the default from swagger-ui to scalar it would be possible to make the documentation UI pluggable where the user can choose either (or other) or just disable it entirely?

[marclave]

I wonder if rather than shifting the default from swagger-ui to scalar it would be possible to make the documentation UI pluggable where the user can choose either (or other) or just disable it entirely?

@mikaelkaron I like your idea of making it configurable! where the user can choose:

• scalar
• swagger ui
• none

im guessing we can keep the configuration inside of platformatic.json? Maybe under a new field called OpenAPIDocumentation which is an object with options of default + route.

[marclave]

also just a heads up we're working on making an alternative layout that's slightly more consistent to swagger UI :)

[mikaelkaron]

Yeah, and perhaps the name could even be an npm package?

How different is this than just using plugins with packages except that you’d like to have a way to remove the default swagger-ui?

@mcollina - would it be possible to have the UI be part of a generator rather than just bundled and have the user able to switch?

[mikaelkaron]

Btw, the same would probably be true for graphiQL

[mcollina]

A few notes:

1. fix the DCO
2. It seems the new plugin is not generating the /documentation/json route (take a look at es.js#L161-L201). Currently it exposes the spec as /spec.json.

---

I really like the new UI.

@mikaelkaron would you prefer the old school UI? If so, why?

I think we could make it configurable somehow / part of the generator.

[socket-security]

New and updated dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
tap	16.3.9	environment	+9	54.9 MB	isaacs
pino	8.16.2	None	+0	687 kB	matteo.collina
pino-pretty	10.2.3	None	+0	219 kB	jsumners
vite	5.0.10...5.0.7	None	+1/-1	5.62 MB	vitebot

[marclave]

A few notes:

1. fix the DCO
2. It seems the new plugin is not generating the /documentation/json route (take a look at es.js#L161-L201). Currently it exposes the spec as /spec.json.

I really like the new UI.

@mikaelkaron would you prefer the old school UI? If so, why?

I think we could make it configurable somehow / part of the generator.

@mcollina Glad you like the new UI ✨

I just pushed up a fix for DCO & also exposing the swagger spec download to the old route.

I think if the preference is the old school UI, we are going to have a configuration where users can choose "modern" or "classic" layout. Hopefully shipped in the next few weeks 🚀

The classic is this one :) Which feels more closer to Swagger UI #1858 (comment)

[mikaelkaron]

would you prefer the old school UI? If so, why?

Nah, I like the new one better (modern even), but I think you'd want to be able to choose if/which SwaggerUI you want.

That's why I'd like it to be part of the generator, but in a way that it just adds a plugins.packages entry to either @fastify/swagger-ui or @scalar/fastify-api-reference if the user want's a UI, or nothing if the user don't want a UI (but still a .json generated with the OpenAPI).

[mcollina]

A few things are missing

• The UI loads /spec.json, which should not be served (at all)
• The /documentation/json endpoint is listed in the UI (see fastify-swagger on how to hide it)
• /documentation/yaml is missing
• terser is added as a dependency and it does not seem used.
• @scalar/api-reference does not seem to be used in production, only to get a type (it also has a massive dependency list)
• it seems the UI is loading a file from a fixed URL (in @fastify/swagger-ui it is relative), which is tricky when composing plugins.
• please avoid the use of console, use the logging facilities.
• prefer the use of return html or return reply.send(html) to avoid forking the promise chain. Note that in those cases both handlers do not need to be async.

[marclave]

Appreciate the review @mcollina ; we will fix this up :)

[marclave]

A few things are missing

• The UI loads /spec.json, which should not be served (at all)
• The /documentation/json endpoint is listed in the UI (see fastify-swagger on how to hide it)
• /documentation/yaml is missing
• terser is added as a dependency and it does not seem used.
• @scalar/api-reference does not seem to be used in production, only to get a type (it also has a massive dependency list)
• it seems the UI is loading a file from a fixed URL (in @fastify/swagger-ui it is relative), which is tricky when composing plugins.
• please avoid the use of console, use the logging facilities.
• prefer the use of return html or return reply.send(html) to avoid forking the promise chain. Note that in those cases both handlers do not need to be async.

@mcollina just pushed up fixes, really appreciate your time and insights. All of these improvements really make the fastify-scalar integration way better! So we really appreciate that ✨

• The UI loads /spec.json, which should not be served (at all)
  Removed, we have a download button now
• The /documentation/json endpoint is listed in the UI (see fastify-swagger on how to hide it)
  Done :)
• /documentation/yaml is missing
  Added ✨
• terser is added as a dependency and it does not seem used.
  Removed
• @scalar/api-reference does not seem to be used in production, only to get a type
  Removed
• it seems the UI is loading a file from a fixed URL (in @fastify/swagger-ui it is relative), which is tricky when composing plugins.
  Great call, we updated this and can be seen here
• please avoid the use of console, use the logging facilities.
  Fixed :)
• prefer the use of return html or return reply.send(html)
  fixed

[mcollina]

The Node.js recommended client is node-fetch. It's better to recommend fetch() itself.

[mcollina]

There are still some console.logs:

• https://github.com/scalar/scalar/blob/main/packages/fastify-api-reference/src/fastifyApiReference.ts#L120
• not fixed yet https://github.com/scalar/scalar/blob/9c880d04c557cbe011cfecdd6fb2a602704bcb2f/packages/fastify-api-reference/src/fastifyApiReference.ts#L243-L246

[mcollina]

This also needs a rebase (sorry about it)

[hanspagel]

• not fixed yet https://github.com/scalar/scalar/blob/9c880d04c557cbe011cfecdd6fb2a602704bcb2f/packages/fastify-api-reference/src/fastifyApiReference.ts#L243-L246

We have two routes, one for HTML (which is using reply.html() now) and to serve the JavaScript (which is still using reply.send()). Or is there a better way to serve the JavaScript file?

[marclave]

@mcollina

we removed node-fetch, I also tested every http snippet request and they worked. Was just node-fetch.

We removed the stray console log.

@hanspagel left a comment on this

not fixed yet https://github.com/scalar/scalar/blob/9c880d04c557cbe011cfecdd6fb2a602704bcb2f/packages/fastify-api-reference/src/fastifyApiReference.ts#L243-L246

[mcollina]

I've found two issues. The first is that all the Node.js snippets are for old libraries that I would not recommend:

1. axios
2. node:http
3. request

What should be there is:

1. undici https://undici.nodejs.org/#/
2. fetch()

What's the snippet library are you using? we can likely add stuff there.

The second issue is about the Web Client. You are adding all optional query parameters by default. This is a major difference compared to Swagger UI, where those are not added if not set.

[marclave]

I've found two issues. The first is that all the Node.js snippets are for old libraries that I would not recommend:

1. axios
2. node:http
3. request

What should be there is:

1. undici https://undici.nodejs.org/#/
2. fetch()

What's the snippet library are you using? we can likely add stuff there.

The second issue is about the Web Client. You are adding all optional query parameters by default. This is a major difference compared to Swagger UI, where those are not added if not set.

...What's the snippet library are you using? we can likely add stuff there.

Appreciate the review! We are using httpsnippet-lite https://github.com/P0lip/httpsnippet/, will look into adding undici + fetch

... You are adding all optional query parameters by default. ...

Ah, yes we can fix this ✨

[marclave]

@mcollina we fixed the "... You are adding all optional query parameters by default. ..." issue :)

as for the "Node.js snippets are for old libraries" comment, we realized that adding the two additional libraries is a larger body of work, but we have started the undici snippet library here :)

would be excited to fast follow with the undici + fetch snippet library after merge. But if you want those two snippet libraries finished first let me know :)

[marclave]

@mcollina I bring good news :) the wonderful @hanspagel pushed up the undici code snippets into Scalar

[mcollina]

lgtm

[marclave]

@mcollina looks like i was missing @platformatic/scalar-theme from the package.json in the composer package.

the tests that were failing should run now so we can kick those off again ✨

[marclave]

@mcollina i fixed the failing ci-db-core-test

im not sure on ci-service ci-runtime ci-composer & playwright-e2e failing in this branch (also appears to be on main?) let me know on any guidance 💫

[mcollina]

yeah, we had a few CI problems.

[mdgozza]

Is it possible that this feature brings back the annoying spamming logs for requests to the documentation/json resource that is caused by a Compose Service polling?

I had hoped those logs went away with this issue:

#1083 (comment)

I can't find a convenient way to disable them exclusively for that resource otherwise.