🦋 Changeset detected

Latest commit: 0ef72f8

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

render.js is a shocking mess and this makes it worse. would be good to spend some time refactoring it soon

Converted back to draft as there's a flaw in this approach — as far as Rollup is concerned, things are still unstable (i.e. if a chunk that imports ./other-chunk-123.js changes to ./other-chunk-456.js as a result of other-chunk changing, the chunk itself will get a different hash).

So we need to make sure that we, rather than Rollup, control the hashing. Gah

Alright, fixed. Rollup controls the hashing if we're not using import maps, otherwise we do. It works beautifully

There's a Vite PR that implements using importmaps too: vitejs/vite#15373. Could be useful to cross-reference and even work on something builtin.

We could probably leave the entry chunk out of the import map as I don't believe there'd be any benefit to including it

This already happens — the app and start entry points are automatically excluded, because we only add things to the import map if they're actually imported. If you're thinking of chunks/entry, that's not an entry chunk (though perhaps it should have a better name).

Though I just realised that the start entry point is basically just this:

import{c as a}from'chunks/entry';export{a as start};

The reason for that is so that other stuff in chunks/entry can be treeshaken, but it would be nice if that didn't result in two separate modules. Seems like something that could maybe be fixed with a tweaked configuration.

One related note — the entry chunk is always invalidated because it references __sveltekit_xyz213 (the name includes a hash of the version string). I'd like to fix that as part of this work, or maybe as a separate PR.

I also wonder a bit whether we want to include the nodes for each +page.svelte in the import map or not

Ooh, interesting. One consideration here is that we always need to load the entry chunk before doing any other work, so making that chunk more stable is helpful.

We could increase its stability (and decrease its size) relative to its current state (but not relative to the case where nodes are included in the import map) by separating out the things it contains — the route manifest, re-exported hooks, and a re-exported root.svelte (which depends on svelte/internal which will sometimes change between versions).

It's very hard to know exactly which levers to pull here! I guess we could make it configurable (which is why I went for importMap.enabled instead of just an importMap boolean) though optionitis is an unpleasant disease. We can also vary the defaults as we learn more, I guess.

There's a Vite PR that implements using importmaps too: vitejs/vite#15373

Oh, interesting. Would definitely be nice to use something built-in, especially since this PR is doing some slightly hacky stuff after the bundle has been created. Some thoughts on that PR:

• I'm not 100% sure it will actually work in its current state? It's using a similar approach to the one I initially tried, namely replacing the !~{...}~ placeholders with stable strings inside renderChunk. But that doesn't affect how Rollup calculates the hash, so even though the outputted code has stable identifiers, you still get cascading invalidation. I could be misunderstanding something though
• it assumes Vite is transforming your index.html, which doesn't apply with appType: 'custom' — we need to be able to generate the import map dynamically at SSR time (since it uses relative paths)
• I guess we need to think about sourcemaps 😭

Oh, interesting. Would definitely be nice to use something built-in, especially since this PR is doing some slightly hacky stuff after the bundle has been created. Some thoughts on that PR:

Thanks for reviewing that. I haven't quite dig into it, but seems like you're right with that about the !~{...}~ part 🤔 Maybe we need a new flag if it's built into Vite so it works like this implementation.

• we need to be able to generate the import map dynamically at SSR time (since it uses relative paths)

I'm thinking Vite could expose some additional information in the SSR manifest that may help with this. But good point too so we make sure SSR frameworks can make use of this too.

Thank you both for reviewing! 😀

it will actually work in its current state?

Yes, I've checked it and it does work!

# Vite repo, switch to bhbs:chunkMap
# pnpm build
cd playground/dynamic-import
pnpm run build | sort > before.log
sed -i '' "s/'hello'/'hi'/" nested/hello.js
pnpm run build | sort > after.log
diff before.log after.log

11c11
< dist/assets/hello-RIS-8H9F.js   0.09 kB │ gzip: 0.10 kB │ map: 0.21 kB
---
> dist/assets/hello-FkIcqXXg.js   0.09 kB │ gzip: 0.10 kB │ map: 0.21 kB

After replacing with static IDs, the hash calculation is performed on the altered content, creating a situation where the content hash of the entry itself does not change. This is my understanding 🤔

source-map

If we handle things in renderChunk inside Vite, as long as we properly process the sourcemap at that step, we can leave the rest to Rollup. For now, I've confirmed that the existing sourcemap tests in Vite are passing.

I'm thinking Vite could expose some additional information in the SSR manifest that may help with this.

Sounds good 😀

@Rich-Harris I'm closing this because it has numerous conflicts and probably would be easier to start over with -- also, the Vite PR doing the same thing hasn't merged yet (last activity about a year ago). Happy to re-pursue this sometime in the future, or you can reopen it if there's a reason to keep it open.