search

WebKit / Speedometer

An open source repository for the Speedometer benchmark
Other
592 stars 68 forks source link

[fix] News Site Nuxt: rename 'className' -> 'class' #400

Closed flashdesignory closed 5 months ago

flashdesignory commented 5 months ago

Two instances, where className instead of class was used. This happened in areas that tests don't cover and shouldn't impact score.

More details: on small screens the news site hides the navbar and displays a hamburger icon. This hamburger icon had some missing class assignments due to the oversight. Since we don't test mobile view, this is not an issue in the current tests.

@kara

rniwa commented 5 months ago

It's good to fix this for the next version but I don't think we should merge this back into 3.0 at this point.

flashdesignory commented 5 months ago

It's good to fix this for the next version but I don't think we should merge this back into 3.0 at this point.

yeah, no need to merge into 3.0.

flashdesignory commented 5 months ago

I'm surprised that for a patch that should change nothing there are so many things changed in the rendered files. Can you check why this happens? Also it looks like some dist files were removed by mistake...

Oh you're so right 🤦 .... To create static files, we need to use the generate script and not the build script. It's confusing and we should probably change that at some point..

flashdesignory commented 5 months ago

@julienw - regarding className vs class: this is more a change to be consistent. I don't think it's ideal to use both in the same codebase. You're right that it doesn't change the behavior, it's really just clean-up!

julienw commented 5 months ago

@julienw - regarding className vs class: this is more a change to be consistent. I don't think it's ideal to use both in the same codebase. You're right that it doesn't change the behavior, it's really just clean-up!

Thanks for confirming; I don't mind doing cleanup, but your PR description was mentioning class assignments were missing, so I was curious if I missed anything here.

I'm still surprised that there are so much changes in the generated files despite the change doing nothing... It would be good that we can make this more predictable eventually...

flashdesignory commented 5 months ago

I'm still surprised that there are so much changes in the generated files despite the change doing nothing... It would be good that we can make this more predictable eventually...

yeah, nuxt generates a new hash that's being used in the filename... I'll look into it separately.

julienw commented 5 months ago

yeah, nuxt generates a new hash that's being used in the filename... I'll look into it separately.

It's not just the hash, if you look at the content of the files, the minified variable names are also different... that's so weird.