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

Feat/spec renderer resolved #8028

Closed
wants to merge 3 commits into from

Conversation

jackkav
Copy link
Contributor

@jackkav jackkav commented Sep 30, 2024

Motivation: use internel spec renderer, over swagger-ui-dist
Aim: drop-in replacement
Todo:

  • add and use @kong/spec-renderer
  • remove swagger-ui-dist
  • point at spec-renderer stable build
  • confirm behaviour and layout meets aim
  • scroll overflow
  • color themeing decision, either dark/light or take something similar to our css variable palette

Future work

  • use bundler type module resolution in insomnia

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@adamdehaven
Copy link
Member

color themeing decision, either dark/light or take something similar to our css variable palette

@jackkav for now, we'd recommend utilizing the default theme (light) in the short term. There is theming available currently via CSS Custom Properties; however, we're going to bake-in easier theming and possibly a default dark theme that you can then tweak for Insomnia specifically. We're working with Design on getting some "official" themes, so recommended to hold on working on customizing on your own in the short-term

@jackkav
Copy link
Contributor Author

jackkav commented Oct 2, 2024

I was trying to fix the styling and I encountered a couple of dx issues,

  1. the vite dev server started opening a chrome tab on save
  2. a new error is appearing in the console Warning: You are calling ReactDOMClient.createRoot() on a container that has already been passed to createRoot() before. Instead, call root.render() on the existing root instead if you want to update it.

@adamdehaven
Copy link
Member

adamdehaven commented Oct 2, 2024

I was trying to fix the styling and I encountered a couple of dx issues,

  1. the vite dev server started opening a chrome tab on save

I'm not sure on this one; nothing in the web component should be triggering this?

  1. a new error is appearing in the console Warning: You are calling ReactDOMClient.createRoot() on a container that has already been passed to createRoot() before. Instead, call root.render() on the existing root instead if you want to update it.

This typically happens when a DOMContentLoaded event listener is triggered twice. Is there any chance this line is getting called more than once?

Additionally, do you need to unmount the web component at some point, e.g. root.unmount()?

@@ -42,6 +42,7 @@
"@getinsomnia/node-libcurl": "^2.4.31-0",
"@grpc/grpc-js": "^1.10.10",
"@grpc/proto-loader": "^0.7.13",
"@kong/spec-renderer-dev": "^1.76.4-pr.339.fdf5330.0",
Copy link
Member

Choose a reason for hiding this comment

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

We merged in the changes from this PR preview into main for now (still need to resolve exports, etc.) but you can install the latest

npm i @kong/spec-renderer-dev@latest

@jackkav
Copy link
Contributor Author

jackkav commented Oct 2, 2024

Both issues could be caused by the place we register I can try a few other places that are more likely to only eval once

@jackkav
Copy link
Contributor Author

jackkav commented Oct 3, 2024

above idea didn't work, looks like the spec renderer register function is interfering with vite HMR in insomnia.
moving discussion to #8036

@jackkav jackkav closed this Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants