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

[benchmark] Fix benchmark scripts & moved scenarios to correct benchmark project #23058

Merged
merged 4 commits into from Oct 14, 2020

Conversation

mnajdova
Copy link
Member

@mnajdova mnajdova commented Oct 14, 2020

Fixes issue with running scripts in the material-ui-benchmark after introducing #22923

image

Also, as agreed in #22923 the scenarios using component rendering are moved to the browser benchmarks, while the the ones which tests only javascript functions are moved to the material-ui-benchmark.

In addition, the yarn benchmark script was renamed to yarn benchmark:browser to better reflect what the benchmarking is about.

@mui-pr-bot
Copy link

mui-pr-bot commented Oct 14, 2020

No bundle size changes comparing 888614b...e7f8716

Generated by 🚫 dangerJS against e7f8716

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

Did you consider this fix?

diff --git a/babel.config.js b/babel.config.js
index ac044fb372..b5f0c1cd97 100644
--- a/babel.config.js
+++ b/babel.config.js
@@ -137,7 +137,6 @@ module.exports = function getBabelConfig(api) {
      benchmark: {
        plugins: [
          ...productionPlugins,
           [
             'babel-plugin-module-resolver',
             {
-              root: ['./'],
               alias: defaultAlias,
             },
           ],

I think that we should try to unify these two benchmarks under the same folder. I think that specializing /benchmark into /benchmark-browser doesn't immediately help. I guess, later on, we can do /benchmark/browser, /benchmark/js and /benchmark/server. Maybe we could start folder nesting right now (/benchmark/browser)?

In hindsight, I think that I created /packages/material-ui-benchmark/ in the first place to be able to leverage custom dependencies without impacting the main package.json. I'm not sure if it was really wise as a developer would install the whole workspace anyway.

@mnajdova
Copy link
Member Author

I think that we should try to unify these two benchmarks under the same folder. I think that specializing /benchmark into /benchmark-browser doesn't immediately help. I guess, later on, we can do /benchmark/browser, /benchmark/js and /benchmark/server. Maybe we could start folder nesting right now (/benchmark/browser)?

That makes more sense, let me change the folder structure 👍

@mnajdova mnajdova merged commit ab6f24d into mui:next Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants