Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove swagger-ui #672

Merged
merged 19 commits into from
Oct 10, 2022
Merged

Remove swagger-ui #672

merged 19 commits into from
Oct 10, 2022

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Sep 30, 2022

Checklist

@kibertoad
Copy link
Member

What is the rationale behind this change?
This part of fastify-swagger is very widely used

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Sep 30, 2022

We want to remove swagger-ui from fastify-swagger and create a new plugin fastify-swagger-ui which has the swagger-ui part.
see fastify/fastify-swagger-ui#1

So based on that you can have e.g. a fastify-rapidoc impementation where you use fastify-swagger for swagger file generation and rapidoc as openapi viewer.

@kibertoad
Copy link
Member

Makes sense. Then probably we first should release fastify-swagger-ui before this lands?

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Sep 30, 2022

I am still missing 3 code branches for 100% test coverage.

@kibertoad
Thats why I point from initial implementation to this git branch. Also have some deactivated unit tests in fastify-swagger-ui, which have to work again.

@kibertoad
Copy link
Member

We need to document migration path to fastify-swagger-ui somewhere

@mcollina
Copy link
Member

mcollina commented Oct 2, 2022

@fastify/swagger-ui is out. Could you update this PR, make the CI pass, and add a migration plan?

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Oct 5, 2022

So! It seems that it is done code wise.

Now regarding the documentation: How should it be done?

I thought of:

  • Create a Migrate_7_To_8.md (suggestion regarding name welcome)
  • Short Section in readme.md regarding integration with @fastify/swagger-ui, reference to the migration.md

Do you agree? Anything that I miss?

Btw. code review is welcome.

@kibertoad
Copy link
Member

Create a Migrate_7_To_8.md (suggestion regarding name welcome)

Could be UPGRADE.md or MIGRATION.md

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Oct 7, 2022

I created the migration documentation. So please review it :)

@Uzlopak Uzlopak requested a review from darkgl0w October 9, 2022 10:32
MIGRATION.md Outdated Show resolved Hide resolved
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Oct 10, 2022

@mcollina PTAL

MIGRATION.md Outdated Show resolved Hide resolved
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Oct 10, 2022

So when we merge this and release, then we should do the follow up:

  1. Publish @fastify/swagger as 8.0.0
  2. merge Next fastify-swagger-ui#6
  3. Publish @fastify/swagger-ui

It is utmost important to release a new version of @fastify/swagger-ui.

MIGRATION.md Outdated Show resolved Hide resolved
MIGRATION.md Outdated Show resolved Hide resolved
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Oct 10, 2022

@mcollina

Using now import for top level await.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Uzlopak Uzlopak merged commit f15bebd into master Oct 10, 2022
@Uzlopak Uzlopak deleted the ui-patch branch October 10, 2022 14:47
@gspetrou
Copy link

Rando here who was really confused trying to set this up: @fastify/swagger 8.0 is published but @fasitfy/swagger-ui is not. Any ETA?

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Oct 10, 2022

@gspetrou
I asked @mcollina . He wrote he will do it asap. But it might be tomorrow.

I apologize for the inconvenience.

@gspetrou
Copy link

No problem, impeccable timing on my part! Thanks

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Oct 10, 2022

@gspetrou
It got released 🙏

@Fdawgs
Copy link
Member

Fdawgs commented Oct 18, 2022

Thanks for working on this btw @Uzlopak. Never used the UI part in my APIs and it added an extra ~4MB to the build, which for some accounted for 20% of their size!

@briceruzand
Copy link
Contributor

Great feature, because as you, I was not using swagger-ui.
Until now, I was exposing my dynamic openapi spec via swagger at /documention/json to use it with rapidoc ui.
But now routePrefix and exposeRoute options have been removed, how can I expose my dynamic openapi json spec with fastify-swagger ?

Thx

https://github.com/fastify/fastify-swagger/pull/672/files#diff-7aa4473ede4abd9ec099e87fec67fd57afafaf39e05d493ab4533acc38547eb8L62-L71

@mcollina
Copy link
Member

You'll need to define the routes yourself:

https://github.com/fastify/fastify-swagger-ui/blob/master/lib/routes.js#L95-L115

@dennys
Copy link

dennys commented Sep 28, 2023

routePrefix

Hi, I can use fastify-swagger with fastify-swagger-ui now, and I also can use RapicDoc to get the json.
But I have the same question, how to expose my dynamic openapi json without fastify-swagger-ui.
I try to read routes.js, but still have no idea. Could you give me some hits?

@Eomm
Copy link
Member

Eomm commented Sep 28, 2023

Please open a new detailed issue to be useful for others

Spoiler: fastify.swagger()

@fastify fastify locked as resolved and limited conversation to collaborators Sep 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants