Skip to content

Commit

Permalink
Improve attribute replacement and make it work even if app devs have …
Browse files Browse the repository at this point in the history
…custom preprocessing config.

Auto-reloading doesn't work though, so that needs to be fixed, along with regex specificity.
  • Loading branch information
Greenheart committed Aug 22, 2022
1 parent ee56dc7 commit ca440d2
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 67 deletions.
8 changes: 4 additions & 4 deletions packages/kit/src/core/config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,16 @@ function process_config(config, { cwd = process.cwd() } = {}) {

const replaceSveltkitAttributes = preprocess({
// TODO: Update regex to only match attributes and not all string values, to prevent it from replacing text content.
// replace: [[/sveltekit\:prefetch/g, 'data-sveltekit-prefetch']]
// TODO: Ensure the replacement happens for prerendered HTML too
replace: [
[/sveltekit:attribute/g, 'data-sveltekit-attribute'],
[/sveltekit\:prefetch/g, 'data-sveltekit-prefetch']
[/sveltekit\:prefetch/g, 'data-sveltekit-prefetch'],
[/sveltekit\:reload/g, 'data-sveltekit-reload'],
[/sveltekit\:noscroll/g, 'data-sveltekit-noscroll']
]
});

// TODO: Update types of `config.preprocess` to get type safety here. Use correct typing and validation from vite-plugin-svelte
// TODO: This needs to support all valid configs for preprocess, possibly including objects and not just arrays.
// TODO: Ensure the svelte.preprocess API can run multiple instances of `svelte-preprocess` if the user has their own config too.
if (Array.isArray(validated.preprocess)) {
validated.preprocess.push(replaceSveltkitAttributes);
} else {
Expand Down
4 changes: 2 additions & 2 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -1122,7 +1122,7 @@ export function create_client({ target, base, trailing_slash }) {
if (
a.hasAttribute('download') ||
rel.includes('external') ||
a.hasAttribute('sveltekit:reload')
a.hasAttribute('data-sveltekit-reload')
) {
return;
}
Expand All @@ -1149,7 +1149,7 @@ export function create_client({ target, base, trailing_slash }) {

navigate({
url,
scroll: a.hasAttribute('sveltekit:noscroll') ? scroll_state() : null,
scroll: a.hasAttribute('data-sveltekit-noscroll') ? scroll_state() : null,
keepfocus: false,
redirect_chain: [],
details: {
Expand Down
55 changes: 4 additions & 51 deletions packages/kit/src/vite/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,14 @@ const enforced_config = {
root: true
};

// TODO: Figure out how to reload the svelte.config.js when changes happen.
const svelte_config = await load_config();

/**
* @return {import('vite').Plugin[]}
*/
export function sveltekit() {
// TODO: find out if this is the right place to ensure we add preprocessing for replacing sveltekit specific attributes
// NOTE: OR maybe it is autimatically passed in, since it uses the svelte.config.js file where we might have additional preprocessing
// TODO: Ensure preprocessing works with multiple calls to `svelte-preprocess`.
// Seems like it should be find though, as it likely is using an internal closure to keep state, and returning the callback to actually execute the transform.

// IDEA: We could use svelte-preprocess to create the needed config for preprocessing `sveltekit:*`
// TODO: Update regex to only match attributes and not all string values, to prevent it from replacing text content.
// import preprocess from 'svelte-preprocess'
// preprocess({ replace: [/sveltekit\:prefetch/g, 'data-sveltekit-prefetch'] })

return [...svelte(), kit()];
return [...svelte({ configFile: false, ...svelte_config }), kit()];
}

/**
Expand Down Expand Up @@ -337,46 +330,6 @@ function kit() {
}
},

// IDEA: Maybe the attribute replacement could be done when rendering the chunks to avoid looping an extra time (as is the case with generateBundle).
// /** @type {import('rollup').RenderChunkHook} */
// renderChunk(code, chunk) {
// for (const [attribute, replacement] of [['sveltekit:prefetch', 'data-sveltekit-prefetch']]) {
// if (code.includes(attribute)) {
// console.log(`[${chunk.name}]: Replacing "${attribute}" with "${replacement}"`);

// return code.replace(attribute, replacement);
// }
// }

// return code;
// },

// TODO: Ensure the replacement only happens for attributes
// TODO: Ensure the replacement happens for prerendered HTML too
// This might be read from the svelte source file which means we need to replace it when server side rendering the HTML response too.
/**
* @param {import('rollup').OutputOptions} _options
* @param {{ [fileName: string]: any }} bundle
*/
generateBundle(_options, bundle) {
for (const entry of Object.values(bundle)) {
if (entry.type !== 'chunk') continue;

for (const [attribute, replacement] of [
['sveltekit:prefetch', 'data-sveltekit-prefetch']
]) {
if (entry.code.includes(attribute)) {
console.log(`[${entry.fileName}]: Replacing "${attribute}" with "${replacement}"`);

bundle[entry.fileName] = {
...entry,
code: entry.code.replaceAll(attribute, replacement)
};
}
}
}
},

/**
* Vite builds a single bundle. We need three bundles: client, server, and service worker.
* The user's package.json scripts will invoke the Vite CLI to execute the client build. We
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
<a sveltekit:prefetch href="/path-base/prefetching/prefetched">click me for sveltekit:prefetch</a>
<br />
<a sveltekit:attribute href="/path-base/prefetching/prefetched">click me for sveltekit:attribute</a>
<br />
<a sveltekit:noscroll href="/path-base/prefetching/prefetched">click me for sveltekit:noscroll</a>
<br />
<a sveltekit:reload href="/path-base/prefetching/prefetched">click me for sveltekit:reload</a>
<br />
<a sveltekit:something href="/path-base/prefetching/prefetched">click me for sveltekit:something</a>
17 changes: 7 additions & 10 deletions packages/kit/test/apps/options/svelte.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,13 @@ const config = {
}
},
preprocess: [
// IDEA: We could use svelte-preprocess to create the needed config for preprocessing `sveltekit:*`
// TODO: Update regex to only match attributes and not all string values, to prevent it from replacing text content.
// preprocess({
// // TODO: Find a way to create a combined preprocess() inside of sveltekit.
// // This works perfectly, but we shouldn't have to expose this detail to app developers. There must be a way to combine it.
// replace: [
// [/sveltekit\:attribute/g, 'data-sveltekit-attribute'],
// [/sveltekit\:prefetch/g, 'data-sveltekit-prefetch']
// ]
// })
// Test how combined user preprocessing works together with internal sveltekit preprocessing.
preprocess({
replace: [
[/sveltekit\:attribute/g, 'data-sveltekit-attribute'],
[/sveltekit\:something/g, 'data-sveltekit-something']
]
})
]
};

Expand Down

0 comments on commit ca440d2

Please sign in to comment.