Skip to content
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

Upgrade react-router-dom dependency in the dashboard #7252

Open
alpeb opened this issue Nov 10, 2021 · 18 comments
Open

Upgrade react-router-dom dependency in the dashboard #7252

alpeb opened this issue Nov 10, 2021 · 18 comments
Labels
good first issue help wanted javascript Pull requests that update Javascript code

Comments

@alpeb
Copy link
Member

alpeb commented Nov 10, 2021

We're on 5.3.0 and would like to upgrade to v6, but that's a breaking change that requires manual intervention in our routes handling.

The React-router project has promised a compatibility layer to come up at some point that would avoid having to change things too much, but I feel that's a second-best solution and would prefer to have things written as per the best practice.

For anyone wanting to tackling this, we're glad to provide guidance on the overall setup of the dashboard web app in linkerd (also note that BUILD.md provides lots of details).

@alpeb alpeb added help wanted good first issue javascript Pull requests that update Javascript code labels Nov 10, 2021
@hackpk
Copy link

hackpk commented Nov 16, 2021

I'd like to work on this. Also I need guidance as this will be my first issue.

@alpeb
Copy link
Member Author

alpeb commented Nov 16, 2021

@hackpk Excellent. My recommendation is to first try installing linkerd in your local cluster, then try making it work with a separate web development server as explained on BUILD.md, so that you can see in realtime any changes you make to the web code. Feel free to ask any questions in the #contributors channel in our Slack.

@will-codes-things
Copy link

Is this still needed?

@kleimkuhler
Copy link
Contributor

Yes @will-codes-things we're still interested in upgrading this. If you have any questions about contributing you can still go to the #contributors channel on the Linkerd Slack.

@acald-creator
Copy link

Hello there!

I wanted to reach out and see if anyone has started working on this?

If not, I would like to work on this issue.

@acald-creator
Copy link

In fact, I'm already working through the migration, and also I understand that this will require changes to the React class components, since the new version of react-router-dom is using hooks. I will raise some questions in the Slack channel if I have any.

@AgniveshChaubey
Copy link

Is this issue still opened? If yes, I would like to work on this.

@acald-creator
Copy link

From what I could tell this issue is still open but I'm currently working on this issue.

I didn't receive much feedback from Linkerd team.

@adleong
Copy link
Member

adleong commented Jan 19, 2023

Hi @acald-creator, is there any feedback in particular you're looking for or anything we can do to support you?

@acald-creator
Copy link

@adleong Thanks for responding! No not at this time. I'm hoping to have a PR next week. But I will reach out if I have a question.

@acald-creator
Copy link

bump

Wanted to give an update that I am still working on this. My plan was to have the PR last week, but got sidetracked.

@acald-creator
Copy link

I have a branch that I am working on, but there were some questions asked in the Slack channel which I would like to elaborate and answer in this issue or the PR I will provide.

https://github.com/acald-creator/linkerd2/tree/upgrade/react-router-dom

I want to gather all the info first and some steps taken to make this transition possible.

@acald-creator
Copy link

Still working this, but was delayed from being sick last week.

@Mish2j
Copy link

Mish2j commented Apr 21, 2023

Hi there, I would like to work on this. Should I wait to be assigned or I can start working?

@acald-creator
Copy link

acald-creator commented Apr 21, 2023

I was working on this, but got delayed with other things I needed to work on.

I would like to still work on this, but I think it would be great to have additional help.

The road to migrate from v5 to v6 can be incremental, but there are slight breaking changes in certain instances, especially the main Router portion. v6 have changed how the router is designed so that would require the most work to be done.

One thing to note is that this current repo uses React Class components, and certain things in react-router-dom v6 have switched to using React Hooks, and it probably won't work too well in the React Class components.

@adleong
Copy link
Member

adleong commented Apr 27, 2023

Hi @acald-creator

Do you think it will be possible to upgrade to v6 without switching the project to React Hooks? Based on my understanding, this would be a significant effort to migrate the whole project over so I think we'd prefer to avoid that unless it prevents us from upgrading.

@acald-creator
Copy link

@adleong

Here is the reference to a closed issue regarding the support for Class Components.

remix-run/react-router#8146

Just skimming through, we could do a HOC component, and that will just wrap any of the Class components that utilize the react-router-dom library.

And here is the official guide with a locked closed issue, remix-run/react-router#8753 that walks you through on how to migrate.

I can certainly help with migrating overall, but I can be involved later, if you prefer to avoid this.

@pavanjoshi914
Copy link

@adleong is this issue still open to be assigned?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue help wanted javascript Pull requests that update Javascript code
Projects
None yet
Development

No branches or pull requests

9 participants