Skip to content

feat(js/core): added middleware to reflection api#5044

Open
pavelgj wants to merge 11 commits intopj/gm-impl-middlewarefrom
pj/gm-middleware-reflection
Open

feat(js/core): added middleware to reflection api#5044
pavelgj wants to merge 11 commits intopj/gm-impl-middlewarefrom
pj/gm-middleware-reflection

Conversation

@pavelgj
Copy link
Copy Markdown
Member

@pavelgj pavelgj commented Apr 1, 2026

Checklist (if applicable):

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request extends the ReflectionServer to support the 'middleware' type in the /api/values endpoint and implements logic to invoke toJson on returned values when available. It also adds a new test suite to verify the reflection API. A potential runtime error was identified in the mapping logic where a nullish check is required before accessing properties on values.

@pavelgj
Copy link
Copy Markdown
Member Author

pavelgj commented Apr 1, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces toJson mapping for listValues in both ReflectionServer and ReflectionServerV2, allowing for custom serialization of registered values. It also adds support for the middleware type in the REST API and includes comprehensive tests. Review feedback identifies a breaking change in the REST API where the response format was changed from a map to an array, resulting in data loss and inconsistency with the WebSocket implementation. Additionally, the reviewer recommended making the supported type validation more dynamic to match the flexibility of the V2 server.

@pavelgj
Copy link
Copy Markdown
Member Author

pavelgj commented Apr 1, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the reflection servers to support the 'middleware' type and adds logic to map values using their 'toJson' method if available. The reviewer identified that the newly introduced strict validation for the 'type' parameter in the V2 reflection server is overly restrictive and causes a regression by preventing access to other valid types. It is recommended to remove this strict validation to allow the backend to handle type checking, which would also necessitate updating the corresponding test case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant