-
Notifications
You must be signed in to change notification settings - Fork 48
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
Refactor interceptors as middlewares #72
Conversation
djhi
commented
Jun 4, 2024
•
edited
Loading
edited
- Refactor interceptors as middlewares
- Add a Sinon compatible dataProvider in the example
- Update migration guide
- Documentation
I won't upgrade the README in this PR as I intend to rewrite it in a dedicated one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is one thing that we're missing: the ability to use FakeRest directly (as we do in ra-data-fakerest). I don't see any test for that, and I suspect the abstract class doesn't work in that case.
throw new Response( | ||
JSON.stringify({ | ||
errors: { | ||
title: 'An article with this title already exists. The title must be unique.', | ||
}, | ||
}), | ||
{ | ||
status: 400, | ||
statusText: 'Title is required', | ||
}, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still use a throw Response
here instead of returning an object to test that this still works
throw new Response( | ||
JSON.stringify({ | ||
errors: { | ||
title: 'An article with this title already exists. The title must be unique.', | ||
}, | ||
}), | ||
{ | ||
status: 400, | ||
statusText: 'Title is required', | ||
}, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still use a throw Response here instead of returning an object to test that this still works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more remarks, we're getting there!