Skip to content

Conversation

@PyvesB
Copy link
Contributor

@PyvesB PyvesB commented Oct 11, 2025

We are getting starting up the Shields.io service locally: badges/shields#11421

Only arrays are supported nowadays for the proxy configuration, see webpack/webpack-dev-server#4694 and others.

@types/webpack-dev-server is deprecated:

This is a stub types definition. webpack-dev-server provides its own type definitions, so you do not need this installed.

Let's use the up-to-date types from webpack-dev-server instead.

@netlify
Copy link

netlify bot commented Oct 11, 2025

Deploy Preview for docusaurus-openapi ready!

Name Link
🔨 Latest commit 1e24a8e
🔍 Latest deploy log https://app.netlify.com/projects/docusaurus-openapi/deploys/68f35b17539e0b0008bc0472
😎 Deploy Preview https://deploy-preview-300--docusaurus-openapi.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@chris48s
Copy link
Collaborator

Hello 👋

I'm happy to try and review this. Superficially it looks good in the sense that the tests pass and the preview site works. But I do have some thoughts and questions:

  1. Are you able to give a way to reproduce the original error you've got independently of shields? Could I configure the demo site on this project to give the same error? It looks like this problem has manifested downstream after you've updated [email protected] and also @docusaurus/[email protected]. My guess is maybe this a compatibility issue with the new version of docusaurus core? Perhaps the newest version of docusaurus core bumps the version of webpack-dev-server, causing this issue? If so, would this proposed fix still be backwards-compatibile with versions of docusaurus back to 3.7.x or does it require also bumping the min compatible docusaurus version for this library?

  2. How did you pick up the other type annotation issues?

  3. I think the reason why there are new eslint errors in files which aren't really related to this PR is because we've unintentionally updated ESLint or ESLint-adjacent packages on this branch. In general there seems to be way more stuff changing in the lockfile than I would expect for just switching out @types/webpack-dev-server@^4.7.2 for webpack-dev-server@^5.2.2. Upgrading ESLint + plugins isn't necessarily a bad thing, but it seems like it has been done unintentionally here.

@PyvesB
Copy link
Contributor Author

PyvesB commented Oct 12, 2025

Hello chris :D

Are you able to give a way to reproduce the original error you've got independently of shields? Could I configure the demo site on this project to give the same error? It looks like this problem has manifested downstream after you've updated [email protected] and also @docusaurus/[email protected]. My guess is maybe this a compatibility issue with the new version of docusaurus core? Perhaps the newest version of docusaurus core bumps the version of webpack-dev-server, causing this issue? If so, would this proposed fix still be backwards-compatibile with versions of docusaurus back to 3.7.x or does it require also bumping the min compatible docusaurus version for this library?

I've not tried reproducing the original error independently of Shields. Indeed, I believe the upgrade to @docusaurus/[email protected] is at fault here, things were working fine when I switched back to @docusaurus/[email protected]. Release 3.9.0 did pull in a new major version of webpack-dev-server (see facebook/docusaurus#11410), which is the one that includes the allow only Array for the proxy option change I linked to in my previous message. As a side note, my impression is that the proxy settings were never handled correctly in docusaurus-preset-openapi, see the change I made for packages/docusaurus-plugin-proxy/src/index.ts.

How did you pick up the other type annotation issues?

I think the reason why there are new eslint errors in files which aren't really related to this PR is because we've unintentionally updated ESLint or ESLint-adjacent packages on this branch. In general there seems to be way more stuff changing in the lockfile than I would expect for just switching out @types/webpack-dev-server@^4.7.2 for webpack-dev-server@^5.2.2. Upgrading ESLint + plugins isn't necessarily a bad thing, but it seems like it has been done unintentionally here.

All the upgrades and unrelated changes are the side effect of simply running yarn install. I think part of the problem is that package.json requirements are pretty loose. I'm no expert when it comes to JS package management, but I guess we could tighten them up as an alternative.

@chris48s
Copy link
Collaborator

All the upgrades and unrelated changes are the side effect of simply running yarn install

OK. Quick answer to this bit:
If you revert the package.json and yarn.lock back to they were before and run

yarn workspace docusaurus-plugin-proxy remove @types/webpack-dev-server
yarn workspace docusaurus-plugin-proxy add webpack-dev-server

that will just change that one thing (well 2 things) without updating everything else in the lockfile.

That will also give you a lockfile that is still on @docusaurus/[email protected] (as this branch stands, you've upgraded it to 3.9.1) and that should also make it easier to answer if this change is backwards compatible with docusaurus versions that use the older version of webpack-dev-server.

@PyvesB
Copy link
Contributor Author

PyvesB commented Oct 12, 2025

Thanks for the pro-tips! I've updated the PR accordingly, the changes are much more focused now, with Docusaurus still on 3.8.0.

@chris48s
Copy link
Collaborator

OK, so I tried to test this locally.
Tbh, the proxy feature is something I have never used before but I worked it out this evening. So lets run through how to test it:

First lets check out the main branch.
Second, I'm going to apply this diff to demo/examples/petstore.yaml:

diff --git a/demo/examples/petstore.yaml b/demo/examples/petstore.yaml
index e5c84f6..811888f 100644
--- a/demo/examples/petstore.yaml
+++ b/demo/examples/petstore.yaml
@@ -1,9 +1,4 @@
 openapi: 3.0.0
-servers:
-  - url: //petstore.swagger.io/v2
-    description: Default server
-  - url: //petstore.swagger.io/sandbox
-    description: Sandbox server
 info:
   description: |
     This is a sample server Petstore server.
@@ -176,7 +171,7 @@ paths:
             }
       requestBody:
         $ref: "#/components/requestBodies/Pet"
-  "/pet/{petId}":
+  "/proxy/pet/{petId}":
     get:
       tags:
         - pet

(proxying won't work if we have servers set and pathRewrite: { "^/proxy": "" }, in the settings means it applies to URLs prefixed with /proxy)

Side note: We should probably add an example to the demo site that allows testing the proxy. We can come back to that.. this will do for now.

Now, if you:

You'll get errors:

  • [webpack-dev-server] [HPM] Error occurred while proxying request localhost:3000/pet/100 to http://localhost:8091/ [ECONNREFUSED]
  • Error occurred while trying to proxy: localhost:3000/pet/100

(at least, assuming you don't have anything running on port 8091) but we can see it has attempted to proxy the request as expected 👍

Now if I check out this branch and try the same thing with the same changes to petstore.yaml and do the same thing, I am just getting a 404 Not Found on http://localhost:3000/proxy/pet/100 - it doesn't seem to be trying to proxy the request. So I don't think this has worked 👎


Using that information on how to test can you see if you can get the proxy behaviour working as expected?

@PyvesB
Copy link
Contributor Author

PyvesB commented Oct 18, 2025

Thanks for the pointers! This should now work as expected.

@chris48s
Copy link
Collaborator

OK cool. This latest iteration looks good. Thanks

We will need to call out the new syntax to specify a proxy when we write the changelog for the next release.

In terms of next steps, @jNullj has highlighted another compatibility issue with react v19 and @reduxjs/toolkit in #297 (comment) I suggest we wait to get a patch in for that first and then do a release containing both fixes.

@chris48s chris48s merged commit bbdc892 into cloud-annotations:main Oct 19, 2025
9 checks passed
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.

2 participants