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

Add Vue to update folder #2201

Closed
wants to merge 2 commits into from

Conversation

flashdesignory
Copy link
Collaborator

@flashdesignory flashdesignory commented Sep 21, 2023

This version has been lifted from Speedometer and follows the WebKit styles guide.

For more context regarding this update, please view #2199.

@kara

@FadySamirSadek
Copy link
Collaborator

@momostafas can you help reviewing this?

@FadySamirSadek
Copy link
Collaborator

@flashdesignory Can we get someone from the Vue team to review this?

@yyx990803
Copy link
Contributor

Feedback from Vue team:

Overall, this implementation is a bit outdated, both in terms of the build setup and API usage. If we want the benchmark to be testing something more representative of current AND future Vue usage, here are my suggestions:

  1. Use Vite as the build tool by following the latest documentation here. This ensures the build output is consistent with most new Vue 3 apps being built, and also produces ESM by default. It also avoids the unnecessary target: 'es5' transpilation in the current implementation.

  2. Use the Composition API and <script setup> syntax for components. Again, most new Vue apps are being built using these newer APIs and syntax, and the resulting code may have different performance characteristics. Ideally we want to workload to reflect the latest usage.

@flashdesignory
Copy link
Collaborator Author

Thanks @yyx990803 for the quick review.
I'll take a look and make the suggested updates.

@flashdesignory
Copy link
Collaborator Author

@yyx990803 @FadySamirSadek - closing this pr in favor of #2213

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

Successfully merging this pull request may close these issues.

3 participants