curriculum icon indicating copy to clipboard operation
curriculum copied to clipboard

React router/context: Adjust router component example and mention context earlier

Open MaoShizhong opened this issue 2 years ago • 8 comments

Describe your suggestion

In the React Router lesson, there is an example of refactoring the browser router into its own component which will allow it access to hooks that can be passed into the elements inside it. At the time of writing the lesson, it was said that the docs for React Router had this example as the way to do things.

However, it seems that they have changed how things work rather quietly and now explicitly advise that browser routers should be singletons and not placed inside any component at all, meaning they also will not and should not have direct access to hooks. This is confirmed by the React Router team in this GH discussion and as such, the docs have since been updated to keep the browser router as a singleton outside of any component within what will likely be Main.jsx. Basically just like the current lesson up until the "refactoring to a component" section.

Because it's not within a component anymore, their advice is to manage state for its child components using something like the Context API, which currently is placed after the shopping cart project. Outlets also have an even easier context provider baked in as a prop, callable via useOutletContext (though context will still need to be introduced in some capacity for this).

Because of this now-direct relationship with how the router should be used with context (certainly in regards to the course content), I am proposing that the refactoring to a component part should be removed, a mention that shared state management between elements in the router ought to be handled with Context (which will be covered in a later lesson), then the Context lesson brought back to some point before shopping cart but after router. I am not 100% sure at the moment where I feel the best place for it would be.

Would love to hear some insights on this? I only came to know about this change in the advised implementation from this discord conversation. If not for that, I would not have known and it's likely many others would not have either. @ManonLef has directly encountered issues with state management within the router following the pattern shown in the router lesson as per the conversation following this message.

Path

Node / JS

Lesson Url

https://www.theodinproject.com/lessons/node-path-react-new-react-router and https://www.theodinproject.com/lessons/node-path-react-new-managing-state-with-the-context-api

Checks

(Optional) Discord Name

Mao

(Optional) Additional Comments

No response

MaoShizhong avatar Oct 14 '23 20:10 MaoShizhong

Seems like a good idea to me, any further thoughts @TheOdinProject/react ?

Also thank you for yet another detailed issue!

wise-king-sullyman avatar Oct 16 '23 00:10 wise-king-sullyman

Looks like I missed #26427 before opening this issue!

MaoShizhong avatar Oct 19 '23 14:10 MaoShizhong

Looks like I missed #26427 before opening this issue!

No worries, I won't be able to work on it anyway. So feel free if they've confirmed this issue

fcasibu avatar Nov 04 '23 06:11 fcasibu

I wrote the context lesson with the shopping cart in mind but if it's possibly going to be moved before the shopping cart project do let me know if I can help in some way with creating a different practical example.

fcasibu avatar Nov 06 '23 06:11 fcasibu

I wrote the context lesson with the shopping cart in mind but if it's possibly going to be moved before the shopping cart project do let me know if I can help in some way with creating a different practical example.

Would 100% appreciate the help there if so. Let's see what the maintainers say when they can get to it.

Totally get where you were coming from with the lesson angle but since React router seemed to have changed their docs from when the lessons were written, I can see why Manon had been running into some issues.

MaoShizhong avatar Nov 06 '23 09:11 MaoShizhong

@01zulfi Would you mind giving your opinion on this?

CouchofTomato avatar Nov 06 '23 16:11 CouchofTomato

The suggestions sound good, though I haven't worked with the latest versions of React Router yet. I'll get back to this issue once I have more context/knowledge around the topic

01zulfi avatar Nov 12 '23 09:11 01zulfi

This issue is stale because it has had no activity for the last 30 days.

github-actions[bot] avatar Dec 13 '23 01:12 github-actions[bot]

This issue is stale because it has had no activity for the last 30 days.

github-actions[bot] avatar Mar 15 '24 01:03 github-actions[bot]

@01zulfi

Is this still needed to be reviewed?

CouchofTomato avatar Mar 15 '24 16:03 CouchofTomato

@MaoShizhong what do you think? Does this issue need to be worked on? I'd love if you can take responsibility of resolving this issue (either yourself, or opening it up for contribution), limited bandwidth on my end 😆

01zulfi avatar Mar 15 '24 16:03 01zulfi

@01zulfi no problem, I'll work on this. It does need working on as the current course kinda directly contradicts what the react router docs and team now advise, and what they advise to use for state management (context) is not taught until after a project that uses something that they say should be used with context. And the current course's instructions actually directly cause issues in some scenarios (encountered by some people already) which is why the router team advise the current context approach.

So we definitely need to do some degree of rearrangement. I'll have a think about what feels most appropriate.

MaoShizhong avatar Mar 15 '24 20:03 MaoShizhong

This is one of the most common issues in the react help channel. I think it would be better to put the context's lessons before the shopping cart project and mention outlet context somewhere in the router lesson.

cakegod avatar Mar 15 '24 21:03 cakegod