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

Using nuxt-image module #30

Open
anton-karlovskiy opened this issue Jan 26, 2021 · 11 comments
Open

Using nuxt-image module #30

anton-karlovskiy opened this issue Jan 26, 2021 · 11 comments

Comments

@anton-karlovskiy
Copy link
Member

anton-karlovskiy commented Jan 26, 2021

@pi0

I might be wrong but I guess there's a bug in using modifiers like so:

for (const image of this.images) {
  image.thumb = this.$img(image.file_path, {
    modifiers: {
      width: this.type === 'poster' ? 370 : 533,
      height: this.type === 'poster' ? 556 : 300
    }
  }).url;
  image.src = this.$img(image.file_path).url;
}

Could you please review the following scenarios?
modifiers
custom-provider
console

When we set width and height directly as props, it seems to work as expected (like so).
props

By taking this workaround d66ae7a, I managed to fix those broken images.

@anton-karlovskiy
Copy link
Member Author

CLS comparison between CSS aspect-ratio and nuxt-image aspect-ratio

PR: #31
Lighthouse CI comparison: https://rocky-taiga-72405.herokuapp.com/app/projects/my-favorite-project/compare/02f8b423-b27a-437b-9f25-d1c6592174ee?compareUrl=http%3A%2F%2Flocalhost%3APORT%2F&baseBuild=4d8e52ab-a3a4-43c7-a9a0-64dc4e9e498a

Before:
GitHub commit: 7a2d846
Vercel URL: https://nuxt-movies-a2ol2sipb.vercel.app/

After:
GitHub commit: 5cb209f
Vercel URL: https://nuxt-movies-j374edudw.vercel.app/

FYI: the CLS has regressed a little and we can see the layout shifting with the naked eyes too.

@pi0 @danielroe cc @addyosmani

@addyosmani
Copy link
Member

Could you help us identify where the CLS regression can be seen above? LHCI appears to show that it held the value. Both the above application URLs (vercel URLs) are not loading for me at the moment either I'm afraid. Known issue? :)

image

image

@pi0
Copy link
Member

pi0 commented Feb 2, 2021

@addyosmani @anton-karlovskiy Would you please add my email to vercel team? (pyapar@gmail.com or pooya@nuxtjs.com)

BTW layout shift until using proper cloud resizer by image module update is expected since tmdb possible sizes are not exactly the ones we want.

@anton-karlovskiy
Copy link
Member Author

@addyosmani
Thank you for letting me know the Vercel error. I didn't know it :)
I debugged it and found the following uncaught exception:
realtime-function

The local build is working as expected but fails on Vercel deployment.
According to the Vercel function logs, it appears that we have some problem with nuxt-image configuration.
Even after setting up nuxt-image, the Vercel deployment used to be working as expected as in https://vercel.com/nuxt-movies/nuxt-movies/d4lvpnplw but from after I touched as in 996e898 (https://vercel.com/nuxt-movies/nuxt-movies/1w083s1f8), the Vercel deployment has failed. I tried reverting the changes but the Vercel keeps failing.
@pi0 Could you please give me advice on it? Thank you.

@anton-karlovskiy
Copy link
Member Author

@addyosmani @anton-karlovskiy Would you please add my email to vercel team? (pyapar@gmail.com or pooya@nuxtjs.com)

BTW layout shift until using proper cloud resizer by image module update is expected since tmdb possible sizes are not exactly the ones we want.

@addyosmani Could you please add @pi0 to the Vercel team so that we can get some help of the current Vercel broken issue from @pi0? Thank you.

@pi0 Regarding the layout shift, the CLS was 0 when we used the CSS aspect-ratio but after removing the CSS aspect-ratio, the CLS became 0.013. In the both cases, we used the same size TMDB images.
Here is before and after.

@danielroe
Copy link
Member

danielroe commented Feb 3, 2021

@anton-karlovskiy Likely will be fixed with serverFiles builder configuration - just add the file it can't find to the array (see builder documentation). Also happy to help if added to team (daniel@roe.dev)

@anton-karlovskiy
Copy link
Member Author

@danielroe Thank you very much.
Could you please give me some summary/code-snippet about how to fix with serverFiles?
Actually I'm blocked due to this Vercel error and @addyosmani may be available after about 7 hours.

@danielroe
Copy link
Member

danielroe commented Feb 3, 2021

Docs are here with snippet: https://github.com/nuxt/vercel-builder#serverfiles

Just add the file/folder it's complaining about.

{
  "builds": [
    {
      "src": "nuxt.config.js",
      "use": "@nuxtjs/vercel-builder",
      "config": {
        "serverFiles": ["utils/**"]
      }
    }
  ]
}

@anton-karlovskiy
Copy link
Member Author

@danielroe It's working. Thank you! cc @addyosmani

@anton-karlovskiy
Copy link
Member Author

@pi0 @danielroe

Is preconnect (https://web.dev/preconnect-and-dns-prefetch/) integrated into nuxt-image module so that we would not have to manually add preconnects to image CDN?
cc @addyosmani

@danielroe
Copy link
Member

@anton-karlovskiy I think that's a good idea, and that it should be integrated into nuxt/image. Individual providers could register their own preconnects, etc. Not sure if it's on the roadmap, but I'll raise an issue on that repo: nuxt/image#178

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

No branches or pull requests

4 participants