nuxt-modules / sanity

Sanity integration for Nuxt
https://sanity.nuxtjs.org
MIT License
211 stars 35 forks source link

Difference in resources loaded on client when having visual editing configured #1024

Open jesperbjerke opened 5 days ago

jesperbjerke commented 5 days ago

As per request from @rdunk in https://github.com/nuxt-modules/sanity/pull/1023, here's more info on what I've found in relation to resources loaded when having visual editing configured.

Wasn't sure what to label this as, not really a bug and not really a feature request πŸ˜…

I might be totally off here and just looking at this from the wrong angle, but to me, it looks like that when having visualEditing section configured in the nuxt.config.ts, the page will load with quite a lot more resources.

What I did is create a blank Nuxt project with this module and set up a page that loads a document from Sanity. That page is using the useSanityQuery. In my Nuxt config, I'm just toggling the visualEditing section on/off, running a build and then opening the page with nuxi preview. I have not gone into Sanity Studio (not even running it) and the Presentation tool. I am just loading this page as any regular non-editor user would.

With visual editing configured

sanity: {
    projectId: process.env.SANITY_PROJECT_ID,
    dataset: process.env.SANITY_DATASET,
    minimal: false,
    useCdn: true,
    apiVersion: '2024-05-30',
    visualEditing: {
      token: '<token here>',
      studioUrl: 'http://localhost:3333',
    },
  },

Results in loaded with visual editing

Without visual editing configured

sanity: {
    projectId: process.env.SANITY_PROJECT_ID,
    dataset: process.env.SANITY_DATASET,
    minimal: false,
    useCdn: true,
    apiVersion: '2024-05-30',
  },

Results in loaded without visual editing

What I think this stems from is the way the module is set up. We are checking the visual editing config in build time and swapping out the client composables with those meant for visual editing. These composables import additional dependencies and therefore will cause more resources to be loaded on the client no matter if you're actually using visual editing at the moment or not.

The best scenario would be if we'd only load the additional resources when visual editing has been enabled. Only way to achieve this currently, is to deploy two versions of the same site. One "public production build" for all normal traffic where visual editing config is removed, and one "editing production build" that Sanity Studio would connect to and has the visual editing config set up.

In addition, if we can figure out a way to load the visual editing stuff conditionally, it would open up for further optimisation by allowing us to use the "minimal client" (minimal: true) when a user is not currently using visual editing.

As a comparison, using the minimal client results in these resources being loaded loaded with minimal client

Let me know if I can help with anything!

Versions

module: 1.11.4 nuxt: 3.12.3

rdunk commented 5 days ago

Thank you so much for the thorough report. It's immensely helpful for us when people take the time to raise issues in this way.

I do remember initially trying to support what you're describing re: minimal client, but there are definitely complexities. Mainly the issue of visual editing being enabled at build, but potentially "disabled" at runtime per user. I can't promise too much on the minimal client optimisation for now.

However this dependency issue seems like it could be an oversight somewhere and potentially an easy fix, so I'll definitely do some investigation into that today.

rdunk commented 4 days ago

So I'm not seeing quite the same results as you using our Nuxt starter. I do see a slight increase in the entry size, but the same number of files.

Would you be able to add the following to your Nuxt config and provide similar screenshots (Visual editing enabled/disabled) so I can see the output file names? Thanks!

app: {
  buildAssetsDir: 'assets',
},
vite: {
  $client: {
    build: {
      rollupOptions: {
        output: {
          entryFileNames: 'assets/ve-[name].[hash].js',
          chunkFileNames: 'assets/vc-[name].[hash].js',
        },
      },
    },
  },
},
rdunk commented 4 days ago

Essentially, I can just see the entry file size is increased because of the createQueryStore and defineEncodeDataAttribute imports from @sanity/core-loader in this file.

jesperbjerke commented 4 days ago

Sure!

With visual editing configured

loaded with visual editing (renamed assets)

Without visual editing configured

loaded without visual editing (renamed assets)

With minimal client

loaded with minimal client (renamed assets)

This is the Vue file for the page I'm testing this on

<template>
 <div>
   <h1>{{ data?.title }}</h1>
   <SanityImage v-if="data?.image" :asset-id="data?.image.asset._ref" />
   <SanityContent v-if="data?.body" :blocks="data?.body" />
 </div>
</template>

<script setup lang="ts">
const route = useRoute();
interface Article {
  title: string;
  body: any[];
  slug: string;
  image: any;
}

const query = groq`*[_type == "article" && slug.current == $slug][0]{title, body, slug, image}`
const { data } = useSanityQuery<Article>(query, { slug: route.params.slug });
</script>
jesperbjerke commented 4 days ago

I think it's originating from node_modules/@nuxtjs/sanity/dist/runtime/plugins/visual-editing.client.mjs with the additional imports of

import { useSanityVisualEditingState } from "../composables/visual-editing.mjs"; and import { useSanityVisualEditing, useSanityLiveMode } from "#imports";

and those files in turn also have additional dependencies.

the

if (!useSanityVisualEditingState().enabled)
    return;

Won't really matter in terms of what files to load, because these imports will be included during build time.

rdunk commented 4 days ago

Right, but the enableLiveMode and renderVisualEditing modules are both lazy loaded internally by @sanity/core-loader only when the wrapper functions are called, so although these files will be output at build time, they shouldn't be downloading if visual editing and preview mode are disabled. For reference, this is what I see, with the only change being the entry file as I outlined above:

Visual editing disabled at config level:

Screenshot 2024-07-05 at 12 31 02

Visual editing enabled at config level, disabled at app level:

Screenshot 2024-07-05 at 12 30 22
jesperbjerke commented 4 days ago

Hm, strange that it thinks that visual editing is enabled for me then πŸ€” I'm just using the defaults, no additional code anywhere other than the nuxt config I sent earlier as well as the Vue file.

When you say "disabled at app level" are you adding any additional logic or configuration somewhere?

jesperbjerke commented 4 days ago

I'm using pages: true in the nuxt.config.ts

rdunk commented 4 days ago

That is very strange, I don't think pages: true should make a difference. I'm testing this using our Nuxt template.

Can you check that you aren't seeing any <div class="sanity-visual-editing"> element in the <body> of your application when visual editing is disabled?

jesperbjerke commented 4 days ago

I don't see that div in the root of

Screenshot 2024-07-05 at 14 17 33

Could be nothing, but I see that in your screenshot you're filtering on "JS" in the network tab. If I do that, most of the additional resources in my screenshots are filtered out because they are listed as "text/javascript" (no idea why devtools wouldn't include that in that filter though).

rdunk commented 4 days ago

Ahh right. I see the issue you're describing now! I believe this is Nuxt's default behaviour, to prefetch these scripts, i.e. non-blocking low priority vs. modulepreload. My understanding was that these shouldn't have any meaningful impact on page speeds, but you're right that we don't technically need them to be prefetched. I'll see if I can hook into the build manifest and omit them somehow.

jesperbjerke commented 4 days ago

πŸ‘ On top of that, in my tests I also see a sizeable difference in the document size itself as well as the entry file. But I don't know if that's caused by the bundler splitting the chunks differently. But still a bit weird.

With visual editing configured (see the highlighted rows)

loaded with visual editing (renamed assets) copy

Without visual editing configured (see the highlighted rows)

loaded without visual editing (renamed assets) copy

Tested running a lighthouse score on it as well, got 62 in performance when visual editing was configured and 89 without it.

rdunk commented 4 days ago

Having done some investigation, it looks like we would need to use the Nuxt's ssrContext.head to hook into entries:resolve in order to remove these entries in a server plugin.

However, we have to resolve the filenames themselves in the module's build:manifest hook, but there seems to be no simple way of passing this data from the module to a server plugin. Normally we could set the data on the runtime config, but because we only have access to the data we need inside a hook, that won't work, as addPlugin is called before we can update the config.

It's not pretty, I'll keep looking.