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

TodoMVC workload based on React 18 / React Router / Material UI #373

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

julienw
Copy link
Contributor

@julienw julienw commented Feb 15, 2024

Hi,
I'd like to propose this new workload in tentative/.
It's based on React 18, React Router and Material UI.

Links:

I decided to use Material UI as a way to get deeper React trees.
I also used React Router because it's very much used. The way I used it to control the model might lead to discussions though: I thought of it as a way to "emulate" a server side environment. I believe that if we'd have to implement a TodoMVC app with a server side environment, we could use the same architecture. Here of course I used a local model, not a server.
Because of React 18 (and I believe the CSS-in-JS library used in Material UI) I had to override rAF and setImmediate so that the SP3 runner could capture all the work.
Especially I had to use microtasks (with promises), not normal tasks (with setTimeout) like before. Indeed with just setTimeout, it was happening that we'd get a setTimeout called in another setTimeout, and this was sometimes escaping the benchmark runner.

I also decided to use some more steps in the benchmarks, than in the other TodoMVC workloads. I added:

  • move between "active" and "all" items
  • clear completedi tems

For review, I split this in several commits to make it easier to review, but the bulk of the implementation is in Implement TodoMVC with all its actions.

If we agree on the general approach, but there are small changes to be done, I'd rather land this first and implement the changes in follow-ups. Hopefully this makes sense!

Happy to hear feedback, folks

new BenchmarkTestStep("CompletingSomeItems", (page) => {
const checkboxes = page.querySelectorAll("input[type='checkbox']");

// We'll be checking 5 checkboxes, in different parts of the list.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's 20, not 5

children: [
{
path: "new",
action: async ({ request }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

oh, I like the way the router is set up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!
Yeah, it's the new way introduced in react-router v6.4, I like it as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants