Skip to content

feat(frontend): RFC to incubate React app #2055

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 15, 2021
Merged

feat(frontend): RFC to incubate React app #2055

merged 1 commit into from
Jan 15, 2021

Conversation

jjoyce0510
Copy link
Collaborator

@jjoyce0510 jjoyce0510 commented Jan 12, 2021 •
edited
Loading

Rendered: https://github.com/linkedin/datahub/blob/762ce4a0dcf20ab8ad413f22373dcefcd1ceff3a/docs/rfc/active/react-app/README.md

~ This PR includes a proposal to introduce a React application to be incubated within DataHub ~

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable)

@shirshanka
Copy link
Contributor

Agree with the general direction towards React(w/GraphQL) and proposal to incubate versus boil the ocean.
We have discussed this within LinkedIn several times as well.

jplaisted
jplaisted reviewed Jan 12, 2021
View reviewed changes
nagarjunakanamarlapudi
nagarjunakanamarlapudi reviewed Jan 13, 2021
View reviewed changes
docs/rfc/active/react-app/README.md
Comment on lines +145 to +146
*Why not?*: The intermediate state of a half-react, half-ember app is something we’d rather not think about -- it’s scary & sad. We’d like to avoid degrading client-side developer experience with this type of complexity. Since this migration will take some time, we feel it more productive to iterate independently.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the interim ... Any new development happens in react / ember / both?

  • Developing in both can lead to poor developer experience.
  • Developing in one can lead to drift.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no great answers, once you commit to a dual world for a while.

Here is a strawman that optimizes for autonomous parallel development:

  1. Get to React-Ember functional parity
  2. Recommend migration from Ember -> React
  3. Ember feature freeze
  4. [eventually] deprecate Ember

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this strategy (what flavor of dual development is sustainable) will emerge over time as well. We'll not lose sight of this as we iterate. Thanks @nagarjunakanamarlapudi!

@cptran777
Copy link
Contributor

I think something to note is that an effective incubation of a React app on DataHub should require some modifications and cleanup to the existing Ember code to extract out useful shared modules that are re-usable between the two. Haven't gotten a chance to actually document this yet, but we can have a chat at some point if you'd like to append this RFC with some additional FE/UI information

@shirshanka
Copy link
Contributor

I think something to note is that an effective incubation of a React app on DataHub should require some modifications and cleanup to the existing Ember code to extract out useful shared modules that are re-usable between the two. Haven't gotten a chance to actually document this yet, but we can have a chat at some point if you'd like to append this RFC with some additional FE/UI information

Yes this makes sense. I think the code leverage opportunities will emerge more organically once we start iterating on the React app, thanks for offering to guide us on that @cptran777!

@jjoyce0510
Copy link
Collaborator Author

jjoyce0510 commented Jan 15, 2021 •
edited
Loading

+1! I've called that out explicitly in the FAQs. Agree we should try to share as much as possible w.r.t. UI components!

This doc is not intended to get into specific impl details, so let's chat more offline and make sure we consider this as we iterate.

shirshanka
shirshanka approved these changes Jan 15, 2021
View reviewed changes
Copy link
Contributor

@shirshanka shirshanka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the comments.

@shirshanka shirshanka merged commit 3d5a6c6 into datahub-project:master Jan 15, 2021
@igbopie
Copy link
Contributor

igbopie commented Jan 19, 2021 •
edited
Loading

  • What is the proposed architecture? We all agree on those principles, the devil is in the details.
  • What UI/CSS framework are you going to use? React is only the view part.
  • How is the react scalable easy to override/config?
  • How are you going to maintain code quality standards?
  • What if we don't have the time to migrate some components to react? what is the plan around that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shirshanka shirshanka shirshanka approved these changes

+2 more reviewers

@jplaisted jplaisted jplaisted left review comments

@nagarjunakanamarlapudi nagarjunakanamarlapudi nagarjunakanamarlapudi left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jjoyce0510 @shirshanka @cptran777 @igbopie @jplaisted @nagarjunakanamarlapudi