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

feat: remove SvelteKit specific handling #638

Merged
merged 7 commits into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/smooth-llamas-reflect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@sveltejs/vite-plugin-svelte': minor
---

Remove internal SvelteKit specific handling

- Disallow `kit` prop in inline options
- Remove default `hydratable: true` option for SvelteKit
- Inspector code mounts on `/@vite/client` to be framework agnostic
dominikg marked this conversation as resolved.
Show resolved Hide resolved
9 changes: 0 additions & 9 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -338,15 +338,6 @@ export default {
* inject custom styles when inspector is active
*/
customStyles?: boolean;

/**
* append an import to the module id ending with `appendTo` instead of adding a script into body
* useful for frameworks that do not support trannsformIndexHtml hook
*
* WARNING: only set this if you know exactly what it does.
* Regular users of vite-plugin-svelte or SvelteKit do not need it
*/
appendTo?: string;
}
```

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { getEl, getText, isBuild } from '~utils';
import { getEl, getText, isBuild, page } from '~utils';

describe('inspector-kit', () => {
it('should render page', async () => {
expect(await getText('h1')).toBe('Hello Inspector!');
});
if (!isBuild) {
it('should show inspector toggle during dev', async () => {
await page.waitForLoadState('networkidle');
expect(await getEl('#svelte-inspector-toggle')).not.toBe(null);
});
} else {
Expand Down
35 changes: 4 additions & 31 deletions packages/vite-plugin-svelte/src/ui/inspector/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ export function svelteInspector(): Plugin {
const inspectorPath = getInspectorPath();
log.debug.enabled && log.debug(`svelte inspector path: ${inspectorPath}`);
let inspectorOptions: InspectorOptions;
let appendTo: string | undefined;
let disabled = false;

return {
Expand All @@ -45,13 +44,6 @@ export function svelteInspector(): Plugin {
...defaultInspectorOptions,
...options
};
const isSvelteKit = config.plugins.some((p) => p.name.startsWith('vite-plugin-sveltekit'));
if (isSvelteKit && !inspectorOptions.appendTo) {
// this could append twice if a user had a file that ends with /generated/root.svelte
// but that should be rare and inspector doesn't execute twice
inspectorOptions.appendTo = `/generated/root.svelte`;
}
appendTo = inspectorOptions.appendTo;
},

async resolveId(importee: string, importer, options) {
Expand Down Expand Up @@ -84,32 +76,13 @@ export function svelteInspector(): Plugin {
}
},

transform(code: string, id: string, options?: { ssr?: boolean }) {
if (options?.ssr || disabled || !appendTo) {
transform(code, id, options) {
if (options?.ssr || disabled) {
return;
}
if (id.endsWith(appendTo)) {
return { code: `${code}\nimport 'virtual:svelte-inspector-path:load-inspector.js'` };
}
},
transformIndexHtml(html) {
if (disabled || appendTo) {
return;
if (id.includes('vite/dist/client/client.mjs')) {
return { code: `${code}\nimport('virtual:svelte-inspector-path:load-inspector.js')` };
Copy link
Member Author

@bluwy bluwy Apr 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dynamic import since the Inspector.svelte would rely on /@vite/client too, and it otherwise would cause an import loop and a runtime ReferenceError.

}
return {
html,
tags: [
{
tag: 'script',
injectTo: 'body',
attrs: {
type: 'module',
// /@id/ is needed, otherwise the virtual: is seen as protocol by browser and cors error happens
src: '/@id/virtual:svelte-inspector-path:load-inspector.js'
}
}
]
};
}
};
}
26 changes: 1 addition & 25 deletions packages/vite-plugin-svelte/src/utils/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,7 @@ const allowedPluginOptions = new Set([

const knownRootOptions = new Set(['extensions', 'compilerOptions', 'preprocess', 'onwarn']);

const allowedInlineOptions = new Set([
'configFile',
'kit', // only for internal use by sveltekit
...allowedPluginOptions,
...knownRootOptions
]);
const allowedInlineOptions = new Set(['configFile', ...allowedPluginOptions, ...knownRootOptions]);

export function validateInlineOptions(inlineOptions?: Partial<Options>) {
const invalidKeys = Object.keys(inlineOptions || {}).filter(
Expand Down Expand Up @@ -203,7 +198,6 @@ export function resolveOptions(

removeIgnoredOptions(merged);
handleDeprecatedOptions(merged);
addSvelteKitOptions(merged);
addExtraPreprocessors(merged, viteConfig);
enforceOptionsForHmr(merged);
enforceOptionsForProduction(merged);
Expand Down Expand Up @@ -291,15 +285,6 @@ function removeIgnoredOptions(options: ResolvedOptions) {
}
}

// some SvelteKit options need compilerOptions to work, so set them here.
function addSvelteKitOptions(options: ResolvedOptions) {
// @ts-expect-error kit is not typed to avoid dependency on sveltekit
if (options?.kit != null && options.compilerOptions.hydratable == null) {
log.debug(`Setting compilerOptions.hydratable = true for SvelteKit`);
options.compilerOptions.hydratable = true;
}
}

function handleDeprecatedOptions(options: ResolvedOptions) {
if ((options.experimental as any)?.prebundleSvelteLibraries) {
options.prebundleSvelteLibraries = (options.experimental as any)?.prebundleSvelteLibraries;
Expand Down Expand Up @@ -791,15 +776,6 @@ export interface InspectorOptions {
* inject custom styles when inspector is active
*/
customStyles?: boolean;

/**
* append an import to the module id ending with `appendTo` instead of adding a script into body
* useful for frameworks that do not support trannsformIndexHtml hook
*
* WARNING: only set this if you know exactly what it does.
* Regular users of vite-plugin-svelte or SvelteKit do not need it
*/
appendTo?: string;
}

export interface PreResolvedOptions extends Options {
Expand Down