-
Notifications
You must be signed in to change notification settings - Fork 109
feat: add displayName to Context component #166
Conversation
Hi, @alfdocimo. Welcome to the project. I'm not quite sure about this PR:
|
@alfdocimo are you still interested in implementing this? |
Hey there! Sorry, I've been busy. I am 😁, It's just that maybe this PR should be separated as @stereobooster mentioned. Also, just to clarify, displayName should just have "restful-react" then? export const Context = React.createContext<Required<RestfulReactProviderProps>>({
base: "",
parentPath: "",
resolve: (data: any) => data,
requestOptions: {},
onError: noop,
queryParams: {},
displayName: "restful-react", // is this necesary?
});
export interface InjectedProps {
onError: RestfulReactProviderProps["onError"];
}
export default class RestfulReactProvider<T> extends React.Component<RestfulReactProviderProps<T>> {
public render() {
const { children, ...value } = this.props;
return (
<Context.Provider
value={{
onError: noop,
resolve: (data: any) => data,
requestOptions: {},
parentPath: "",
queryParams: value.queryParams || {},
displayName: "restful-react",
...value,
}}
>
{children}
</Context.Provider>
);
}
} Thanks again, and I hope to make these changes asap (: |
Yeah, maybe Thanks for taking time. |
Updated! 🙏 (Could use some help on creating unit tests for this) Also will create another PR for typos in the readme |
Hiya! any updates on this? 😊 |
Hey @alfdocimo! Thanks for putting the time into this. I think we may not be on the same page about the problem we're trying to solve. What we want to do is not add a configurable In this case, what needs to be done is adding static displayName = "RestfulProviderContext"; above this line, and then everything should be OK. What questions do you have? |
Oh! Thanks for the article @TejasQ, mind = blown 🤯. Yeah, definitely not what I understood. Will add this and create a PR 👍 |
Opened #195 😉 |
Why
Add capabilities to display the name of the Provider and the Consumer as per #107
Also, fixed some typos on the README file 😁
Note / Question:
I was wondering where to add the Unit test for these changes as I see that Context is used in various files but there's not a
Context.test.js
😅
Sorry if my lint made some other changes ):