Current status - Githubissues

node-red / node-red-node-swagger

A set of tools for generating Swagger api documentation based on the HTTP nodes deployed in a flow

Apache License 2.0

62 stars 48 forks source link

Current status #66

Closed JonSilver closed 5 years ago

JonSilver commented 5 years ago

Just wondering about the current status and future of this node.

Swagger-UI <v3 is deprecated.
The last release was 3 years ago.
There are 5 outstanding PRs.
Some past modifications to the code resulted in the current version giving 500 server errors when flows triggered by http-in nodes become complex. For example, an http post triggers a flow, which quickly and dutifully returns a response; meanwhile the flow triggers other parallel paths which can even be (conditionally) circular/recursive. The node-red-node-swagger code tries to follow all these paths, resulting in the errors and rendering the entire thing useless. In fact I've never managed to get it to populate the UI sidebar when the flows get past simple request-response.

The error in the log trace is: TypeError: Cannot read property 'type' of null at checkWiresForHttpResponse

Reverting this one change originally discussed in #17 makes some of the badness go away, but the Swagger UI panel still shows "I see you're lacking Swagger doc..." and although List Operations seems to display endpoints, the details (parameters etc) of each endpoint don't show up.

Happy to contribute if I can.

dceejay commented 5 years ago

Yes - if you are willing to step up and create some PR - then we would be most grateful. Many thanks in advance.

JonSilver commented 5 years ago

I'll see what I can do. Is there a policy document somewhere summarising best practices for node-red development? In particular is there any restriction on rewriting/refactoring old style JS code using ES5/6 enhancements?

knolleary commented 5 years ago

There's no policy as such. As long as the code is clear and supports Node 8 at a minimum then it should be okay.

We try not to refactoring for the sake of refactoring. But I recognise this particular node is long over due some attention. If a refactor helps to tidy it up then that's a good thing.

JonSilver commented 5 years ago

How about browser support? Specifically should we be aiming to make all browser hosted code <=ES5 for IE10 or can we now use ES6 and abandon IE?

dceejay commented 5 years ago

Some users still use IE11 - so ideally that... but... really... not sure the world would end if it wasn't.

JonSilver commented 5 years ago

OK @knolleary & @dceejay, I've got to a point where I'm ready to submit a PR, once I've cleaned up my commit history. I've...

rolled back the commit I mentioned above
fixed all the problems that have prevented my use of the plugin on complex, branching flows for the past 3 years
entirely refactored the code in swagger.js to a modern functional ES6 style with heavy use of destructuring
focused on code readability, maintainability; a more self-documenting, narrative flow with far less wordy code.
left the swagger-ui version at 2.1.4 because of apparent breaking changes in that library from 2.1.4 to 2.1.5 and seemingly at many points since.
applied a modicum of extra styling to swagger-ui-html to help it feel more modern.

The next stage will involve working out how to take the plugin forward to later versions of swagger-ui, which is a whole other ballgame which I don't have time for right now - so collaboration would be greatly appreciated.

dceejay commented 5 years ago

so ok to close this one then...