linkerd / linkerd2

Ultralight, security-first service mesh for Kubernetes. Main repo for Linkerd 2.x.
https://linkerd.io
Apache License 2.0
10.56k stars 1.27k forks source link

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

Open alpeb opened 2 years ago

alpeb commented 2 years ago

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).

hackpk commented 2 years ago

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

alpeb commented 2 years ago

@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 commented 1 year ago

Is this still needed?

kleimkuhler commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

acald-creator commented 1 year ago

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 commented 1 year ago

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

acald-creator commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

Mish2j commented 1 year ago

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

acald-creator commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@adleong

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

https://github.com/remix-run/react-router/issues/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, https://github.com/remix-run/react-router/discussions/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 commented 1 year ago

@adleong is this issue still open to be assigned?