nystudio107 / craft-plugin-vite

Plugin Vite is the conduit between Craft CMS plugins and Vite, with manifest.json & HMR support
MIT License
12 stars 11 forks source link

Generate `<link rel="modulepreload">` directives for entry chunks and their direct imports #2

Closed johndwells closed 3 years ago

johndwells commented 3 years ago

Description

ViteJS has a feature "Preload Directives Generation" that will automatically generate <link rel="modulepreload"> directives for entry chunks and their direct imports:

https://vitejs.dev/guide/features.html#preload-directives-generation

This PR introduces this feature. AsManifestHelper::extractManifestTags() parses the manifest content for the given entry point, it adds a new import type of tag for every file found in the imports key.

Then every tag of type import is output as <link rel="modulepreload">. It also sets crossorigin to the same value as the given entry point.

Note: It does not include files found in the dynamicImports key, as the ManifestHelper::extractCssTags() does. I am not certain if this is the right call or not. But it at least is in line with how Vite does it: https://github.com/vitejs/vite/blob/main/packages/vite/src/node/plugins/html.ts#L283-L295

Note 2: Vite appears to have the ability to include a polyfill for the modulepreload tag, which is not included in this PR. https://github.com/vitejs/vite/blob/main/packages/vite/src/node/plugins/modulePreloadPolyfill.ts

khalwat commented 3 years ago

Thanks for the PR -- I skipped this initially, glad to have it back in.

Did you implement this for the register() methods too?

johndwells commented 3 years ago

@khalwat Ah - commit 4716259 has just been pushed, which now implements on register().

johndwells commented 3 years ago

I've discovered that this generates <link rel="modulepreload"> tags for legacy script imports as well, which means that modern browsers (supporting type="module") still end up downloading any legacy imports. I'm investigating a refactor to address this. Also looking to add the polyfill...

johndwells commented 3 years ago

Commit 582b7a8 is a refactor to prevent adding <link rel="modulepreload"> tags for imports coming from legacy entry files. Refactor covers both the script() and register() functions.

Also introduce polyfill, for browsers that support type="module" only. The polyfill JS is adapted from https://github.com/guybedford/es-module-shims in the same way that Vite has adapted it: https://github.com/vitejs/vite/blob/5c0f0a362f669e3d38ddc51bfdd9d424a7c08feb/packages/vite/src/node/plugins/modulePreloadPolyfill.ts#L33-L100

khalwat commented 3 years ago

Looks great, many thanks!

Jdruwe commented 3 years ago

@johndwells

Note: It does not include files found in the dynamicImports key, as the ManifestHelper::extractCssTags() does.

I am not sure if the Vite plugin actually uses dynamicImports for the css files:

const getCssTagsForChunk = (
  chunk: OutputChunk,
  seen: Set<string> = new Set()
): HtmlTagDescriptor[] => {
  const tags: HtmlTagDescriptor[] = []
  if (!analyzedChunk.has(chunk)) {
    analyzedChunk.set(chunk, 1)
    chunk.imports.forEach((file) => {
      const importee = bundle[file]
      if (importee?.type === 'chunk') {
        tags.push(...getCssTagsForChunk(importee, seen))
      }
    })
  }

  const cssFiles = chunkToEmittedCssFileMap.get(chunk)
  if (cssFiles) {
    cssFiles.forEach((file) => {
      if (!seen.has(file)) {
        seen.add(file)
        tags.push({
          tag: 'link',
          attrs: {
            rel: 'stylesheet',
            href: toPublicPath(file, config)
          }
        })
      }
    })
  }
  return tags
}

Seems to be only using .imports. I tried this in some local vite test project using 2 following files:

main.ts (entry):

import('./other-module')
    .then((module) => {
        console.log('>>> Other module was loaded!');
    });

other-modules.ts

import './dynamic-styling.css'

manifest.json

{
  "index.html": {
    "file": "assets/index.048e877b.js",
    "src": "index.html",
    "isEntry": true,
    "dynamicImports": [
      "src/other-module.ts"
    ]
  },
  "src/other-module.ts": {
    "file": "assets/other-module.65adf9f6.js",
    "src": "src/other-module.ts",
    "isDynamicEntry": true,
    "css": [
      "assets/other-module.19be373f.css"
    ]
  }
}

As you can see the other-module.ts is a isDynamicEntry and it's pointing to assets/other-module.19be373f.css in its css property. In the index.html file I can only see the following script tag being generated by the html plugin:

<script type="module" crossorigin src="/assets/index.048e877b.js"></script>

But if I look into the code of the ManifestHelper.php I can see that the dynamicImports are also taken into account:

...
$imports = array_merge($entry['imports'] ?? [], $entry['dynamicImport'] ?? []);
...
khalwat commented 3 years ago

I am not sure if the Vite plugin actually uses dynamicImports for the css files:

It doesn't, the Vite plugin just scours that to look for generated CSS files. It traverses all of the imports that a file you provide to .register() or .script() uses, and includes any found CSS files in the list of CSS to include for a given page.

Traversing the dependency graph is the only way to find all of the CSS that a given module (or any of its dynamically imported modules) use.

khalwat commented 3 years ago

@Jdruwe My assumption (which may be faulty) was that Vite doesn't actually import the CSS into your document, whether via an entry or via a dynamic import. It just extracts and generates it to files on disk, and you're responsible for adding the styles to the page, whether via a <link rel> tag or some other means.

For JavaScript-based projects where the entrypoint is a .html file it will take care of injecting these for you, but for a backend rendered system like Craft, we need to do it ourselves.

Jdruwe commented 3 years ago

@khalwat Indeed that's what happens, it generates all the files on disk but they have the html plugin generating script and link tags based on the type of import. I think the plugin is used by most web based project. Basically when you start providing you own custom output it stops using the plugin and you are on your own.

output: {
    assetFileNames: 'resources/[ext]/[name][extname]',
    chunkFileNames: 'resources/chunks/[name].[hash].js',
    entryFileNames: 'resources/js/[name].js',
},

I am not using Craft CMS but Adobe Experience Manager (Java in the backend instead of PHP). But I think the end result is the same for both CMS systems. Based on the manifest I can detect which files are dynamicImports and don't need to be included in the link/script generation that happens by the Java backend. I have no idea if that's even possible with Craft CMS but with my CMS the frontend will just request those dynamic files (be it CSS of JS) on the fly when needed.

NOTE: I could also be completely wrong as I am fairly new to Vite, just trying to learn, also I just came across this PR when looking for code examples of people trying to do the same as me ;)

khalwat commented 3 years ago

I am not using Craft CMS but Adobe Experience Manager (Java in the backend instead of PHP). But I think the end result is the same for both CMS systems. Based on the manifest I can detect which files are dynamicImports and don't need to be included in the link/script generation that happens by the Java backend. I have no idea if that's even possible with Craft CMS but with my CMS the frontend will just request those dynamic files (be it CSS of JS) on the fly when needed.

Vite will take care of dynamically loading any JavaScript files that are dynamic imports, so that part of the problem is solved on the frontend.

It would be pretty hard to know when the Vite runtime has loaded a JavaScript dynamic import that also has CSS associated with it, because that can happen at any time (depending on how the import is done via JavaScript -- it could be as soon as the page loads, it could be when a button is clicked, etc.).

And that is all done in JS-land, with no hooks to tell you when it has happened.

So instead we just include all of the CSS that the main entry point or any of its dynamic imports end up using as <link rel> tags on the page.

Jdruwe commented 3 years ago

It would be pretty hard to know when the Vite runtime has loaded a JavaScript dynamic import that also has CSS associated with it, because that can happen at any time (depending on how the import is done via JavaScript -- it could be as soon as the page loads, it could be when a button is clicked, etc.).

In my case this is no issue as the dynamic import just makes another call to the backend and we just return that file be it a css or javascript file. I am using a rollup plugin someone wrote that scans the code and rewrites dynamic import statements: https://www.npmjs.com/package/@aem-vite/import-rewriter to use the correct public path which by default is relative instead of absolute.

khalwat commented 3 years ago

So if they request a JS file, you return it as well as any CSS/assets it imports?

Jdruwe commented 3 years ago

Hmm you are right it seems. Will look into this further. Thanks for the discussion!

Update: I got it working locally but it required some manual work, looking into writing a custom plugin now. Also in your case you can also use the cssCodeSplit: false option, it will gather all css in a single file for you so you don't need to scan the entire manifest file. But I guess you're doing it this way to no force the users into that option.

Jdruwe commented 3 years ago

Update: I got it working, I did not need to write a plugin, just needed to pass the correct rollup FileNames options

https://user-images.githubusercontent.com/4804496/138416855-d7ca84c2-7cd7-43e5-90b8-c5adeb05d73e.mov

It might not make much sense as its related to my CMS but these are my FileNames:

{
    assetFileNames: 'etc.clientlibs/aem-vite-demo/clientlibs/clientlib-esmodule/resources/[ext]/[name].[hash][extname]',
    chunkFileNames: 'etc.clientlibs/aem-vite-demo/clientlibs/clientlib-esmodule/resources/chunks/[name].[hash].js',
    entryFileNames: 'etc.clientlibs/aem-vite-demo/clientlibs/clientlib-esmodule/resources/js/[name].[hash].js',
}

Using this approach I only need to traverse the imports properties and I can leave the dynamicImports alone.

khalwat commented 3 years ago

How are you then extracting all of the CSS that various chunks / dynamic imports might use then?

Jdruwe commented 3 years ago

Vite does that for me out of the box. Ill share the code on Monday, I am on a family weekend now.

Jdruwe commented 2 years ago

In short. If you take a look at the javascript files generated by the rollup build you will see something like this if you are using dynamic import:

document.getElementById("yellow").addEventListener("click", () => {
  g(() => import("../chunks/other-module.40df526e.js"), ["etc.clientlibs/aem-vite-demo/clientlibs/clientlib-esmodule/resources/chunks/other-module.40df526e.js", "etc.clientlibs/aem-vite-demo/clientlibs/clientlib-esmodule/resources/css/other-module.d8b88b27.css", "etc.clientlibs/aem-vite-demo/clientlibs/clientlib-esmodule/resources/chunks/vendor.9c4555cf.js"]);
});

This code is responsible for dynamically importing the other-module and it's imports when clicking on a button. For this to work you need to make sure that the paths are correctly configured:

output: {
    assetFileNames:
      'etc.clientlibs/aem-vite-demo/clientlibs/clientlib-esmodule/resources/[ext]/[name].[hash][extname]',
    chunkFileNames:
      'etc.clientlibs/aem-vite-demo/clientlibs/clientlib-esmodule/resources/chunks/[name].[hash].js',
    entryFileNames:
      'etc.clientlibs/aem-vite-demo/clientlibs/clientlib-esmodule/resources/js/[name].[hash].js',
}

Note that etc.clientlibs is just some AEM specific path that allows resources (js, css, images, fonts...) to be requested from the server.

khalwat commented 2 years ago

Right, very similar to how webpack dynamic imports work. I haven't had any path-related issues on that front, due to the way the plugin works, thankfully!

khalwat commented 2 years ago

Vite does that for me out of the box. Ill share the code on Monday, I am on a family weekend now.

I realize it extracts the CSS for you, but it doesn't load the CSS on the page for you (other than in development) -- you need to include the <link rel> tags on the page for each CSS chunk, no?

Jdruwe commented 2 years ago

It does load the css and js on the page as the output also include code to do so:

 I = function (n, c) {
    return !c || c.length === 0 ? n() : Promise.all(c.map(t => {
        if (t = `${L}${t}`, t in d) {
            return;
        }
        d[t] = !0;
        const e = t.endsWith(".css"), o = e ? '[rel="stylesheet"]' : "";
        if (document.querySelector(`link[href="${t}"] ${o}`)) {
            return;
        }
        const s = document.createElement("link");
        if (s.rel = e ? "stylesheet" : S, e || (s.as = "script", s.crossOrigin = ""), s.href = t, document.head.appendChild(s), e) {
            return new Promise((f, p) => {
                s.addEventListener("load", f), s.addEventListener("error", p)
            })
        }
    })).then(() => n())
};

Basically this script resolves the paths you pass to it as promises and checks the extension and will add the correct tags to the page.

khalwat commented 2 years ago

Oh wow, that's interesting -- I didn't realize it would dynamically load the CSS too. Does it do this for entrypoints as well?

Jdruwe commented 2 years ago

Yes really nice, I am looking which codebase actually adds that part to that output, but did not find it yet, do you have an idea maybe? For the entrypoints I am using Java generated tags as you do in the plugin.

khalwat commented 2 years ago

Nice. There's no major harm in the additional CSS <link rel> tags I'm adding, but probably I should roll that back for dynamic imports, given that it's handled dynamically by the script

khalwat commented 2 years ago

One other thing I'm doing is generating all of the <link rel="modulepreload"> tags that Vite generates for you if you use it in a more "traditional" way.

khalwat commented 2 years ago

So what I'm doing now is traversing the manifest dependency graph for CSS files that are in the main entry or in any of the imports from that main entry, and I've removed dynamicImports since as you noted, the Vite runtime does that for us!

https://github.com/nystudio107/craft-plugin-vite/commit/db82cb207e6de45132208487e37ffae7cff6a37d

Jdruwe commented 2 years ago

Yes, that's exactly what I am doing, please validate if that actually works within craft CMS just to be sure I didn't miss anything :)