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

Performance Enhancements #127

Merged
merged 46 commits into from
Feb 16, 2024
Merged

Performance Enhancements #127

merged 46 commits into from
Feb 16, 2024

Conversation

bryantgillespie
Copy link
Contributor

@bryantgillespie bryantgillespie commented Feb 12, 2024

Grab bag of performance improvements - lots of small tweaks adding up to a big improvement.

  • Use iconify-icon instead of loading the entire material symbols font library
  • Use font fallbacks to prevent CLS
  • Await useAsyncData in dynamic card component to ensure payload.json is generated instead of calling API directly
  • Moving routes for prerendering into separate module for readability / ease of updating
  • Updated to latest version of Nuxt

Old Score
image

New Score
ScreenShot 2024-02-13 at 13 09 27@2x

Copy link

netlify bot commented Feb 12, 2024

Deploy Preview for directus-website ready!

Name Link
🔨 Latest commit 12d9979
🔍 Latest deploy log https://app.netlify.com/sites/directus-website/deploys/65cfda499005af0008aff46b
😎 Deploy Preview https://deploy-preview-127--directus-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@bryantgillespie bryantgillespie changed the title Icon Fix Performance Enhancements Feb 13, 2024
@bryantgillespie bryantgillespie marked this pull request as ready for review February 13, 2024 18:10
@bryantgillespie bryantgillespie requested review from paescuj and phazonoverload and removed request for paescuj February 13, 2024 18:10
Copy link
Member

@paescuj paescuj left a comment

Choose a reason for hiding this comment

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

Great work 🚀 Looks good overall, besides some minor comment, and there seems to be an issue on this specific page https://deploy-preview-127--directus-website.netlify.app/team/ben-haynes where the browser hangs:

Happens in Firefox, on Chrome you'll only notice that it's slow to move away from the page.
Not sure if other pages are affected as well. Let me know if I can assist in debugging this.

components/Base/HsForm.vue Outdated Show resolved Hide resolved
components/Base/Icon.vue Outdated Show resolved Hide resolved
components/Base/Icon.vue Outdated Show resolved Hide resolved
nuxt.config.ts Show resolved Hide resolved
components/Base/CodeSnippet.vue Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@bryantgillespie
Copy link
Contributor Author

Great work 🚀 Looks good overall, besides some minor comment, and there seems to be an issue on this specific page https://deploy-preview-127--directus-website.netlify.app/team/ben-haynes where the browser hangs:

Happens in Firefox, on Chrome you'll only notice that it's slow to move away from the page. Not sure if other pages are affected as well. Let me know if I can assist in debugging this.

Good catch. Not sure why it's hanging there but will get it sorted.

@bryantgillespie
Copy link
Contributor Author

Great work 🚀 Looks good overall, besides some minor comment, and there seems to be an issue on this specific page https://deploy-preview-127--directus-website.netlify.app/team/ben-haynes where the browser hangs:
Happens in Firefox, on Chrome you'll only notice that it's slow to move away from the page. Not sure if other pages are affected as well. Let me know if I can assist in debugging this.

Good catch. Not sure why it's hanging there but will get it sorted.

Looks good now.

Has to be some kind of weird Nuxt issue but if you have a link like /about#core-team like we were using on the /team/[slug] page - it doesn't pickup the cached payload.json for that page. So the delay was because it was refetching all the data for that page client side.

@paescuj
Copy link
Member

paescuj commented Feb 16, 2024

Great work 🚀 Looks good overall, besides some minor comment, and there seems to be an issue on this specific page https://deploy-preview-127--directus-website.netlify.app/team/ben-haynes where the browser hangs:
Happens in Firefox, on Chrome you'll only notice that it's slow to move away from the page. Not sure if other pages are affected as well. Let me know if I can assist in debugging this.

Good catch. Not sure why it's hanging there but will get it sorted.

Looks good now.

Has to be some kind of weird Nuxt issue but if you have a link like /about#core-team like we were using on the /team/[slug] page - it doesn't pickup the cached payload.json for that page. So the delay was because it was refetching all the data for that page client side.

Thanks! Unfortunately, the issue still appears in Firefox. I tested the other core team pages and noticed the same happens on https://deploy-preview-127--directus-website.netlify.app/team/matt-minor:
Screenshot 2024-02-16 at 21 03 54

Could this be somehow caused by a blog post which is listed on both pages?

@bryantgillespie
Copy link
Contributor Author

Who uses Firefox? 🤣

Jokes aside. I'm kinda stumped here. Because it works fine on every other browser.

@paescuj
Copy link
Member

paescuj commented Feb 16, 2024

Who uses Firefox? 🤣

Jokes aside. I'm kinda stumped here. Because it works fine on every other browser.

Happy to take a look at it 🦊👍

@paescuj
Copy link
Member

paescuj commented Feb 16, 2024

Okay, that's the error I'm receiving in dev:

[Vue warn]: Unhandled error during execution of app errorHandler [runtime-core.esm-bundler.js:44:12](http://localhost:3000/_nuxt/node_modules/.pnpm/@vue+runtime-core@3.4.15/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js)
Uncaught (in promise) Maximum recursive updates exceeded in component <Container>. This means you have a reactive effect that is mutating its own dependencies and thus recursively triggering itself. Possible sources include component template, render function, updated hook or watcher source function.

@bryantgillespie
Copy link
Contributor Author

Upstream issue within Vue?

@paescuj
Copy link
Member

paescuj commented Feb 16, 2024

Upstream issue within Vue?

Exactly! Was caused by vuejs/core#10214, can confirm it's fixed now 👍

@bryantgillespie bryantgillespie merged commit b924e8b into main Feb 16, 2024
6 checks passed
@bryantgillespie bryantgillespie deleted the icons-fix branch February 16, 2024 22:05
@bryantgillespie
Copy link
Contributor Author

🙌
Nice find!
Happy to put this one to bed.

Should really help us SEO wise.

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.

None yet

2 participants