[Discuss] Kubeflow Pipelines frontend development best practice.

zijianjoy commented 3 years ago

Topic

Inspired by https://github.com/kubeflow/pipelines/pull/5040#discussion_r569956232 : Determine the pattern and anti-pattern for frontend development on Kubeflow Pipelines. There are some existing development styles which are either out-of-date, or not recommended in the codebase. For example, enzyme vs react-testing-library, Ref vs Hook/State Update.

Request

Everyone is welcome to add their opinions on best practice for frontend development. Please use the following format in your comment for better reviewing:

### Suggested Practice

// Write down the pattern or anti-pattern you suggest, prefer a concise statement with necessary link.

### Reason

// Write down the reason why a pattern is preferred, or why certain pattern should be avoided. You can be very verbose here.

### Example (Optional)

// Give an entry point for people to understand how to adopt this practice, or point out the existing file in codebase where we should improve or should follow.

Action

After collecting feedback, frontend reviewers will make a call on whether to apply such practice, then update the README.

/kind discussion /area frontend

zijianjoy commented 3 years ago

Suggested Practice

Prefer using react-testing-library in testing, avoid using enzyme when possible.

Reason

The more your tests resemble the way your software is used, the more confidence they can give you.

react-testing-library allows you to focus on the user facing elements and interactions, while enzyme has tested implementation detail which might produce false positive/negative when refactoring. Although enzyme has helped with certain scenarios and it has been widely used by community, it can be verbose and fragile which doesn't serve the best interest: production user's experience. However, it is hard to flip the coin at once, so feel free to choose the best fit library depending on the scenario. https://kentcdodds.com/blog/testing-implementation-details

Example (Optional)

https://github.com/kubeflow/pipelines/blob/master/frontend/src/pages/ExperimentList.test.tsx#L317-L324

Instead of testing the props value, validating that the UI has rendered 2 runs for certain experiment is preferred.

Bobgy commented 3 years ago

For snapshot testing best practices, https://github.com/kubeflow/pipelines/pull/3166

Bobgy commented 3 years ago

Regarding the process, I'd suggest putting up individual PRs for the proposals, so that each of them can be addressed and discussed in its own context. It's hard to browse if we have several different topics in one github issue.

Bobgy commented 3 years ago

For one way data flow: https://github.com/kubeflow/pipelines/issues/5136

zijianjoy commented 3 years ago

Regarding the process, I'd suggest putting up individual PRs for the proposals, so that each of them can be addressed and discussed in its own context. It's hard to browse if we have several different topics in one github issue.

That makes total sense to have individual PRs for each proposal, thank you for bringing this up!

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 2 years ago

This issue has been automatically closed because it has not had recent activity. Please comment "/reopen" to reopen it.

zijianjoy commented 2 years ago

/lifecycle frozen

kubeflow / pipelines