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: add rootDir option and set __svelte_meta.file like in svelte4 #11627

Merged
merged 11 commits into from
May 16, 2024
5 changes: 5 additions & 0 deletions .changeset/khaki-mails-scream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

feat: introduce `rootDir` compiler option, make `filename` relative to it
5 changes: 5 additions & 0 deletions .changeset/kind-doors-grin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: rename `__svelte_meta.filename` to `__svelte_meta.file` to align with svelte 4
32 changes: 15 additions & 17 deletions packages/svelte/src/compiler/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ export { default as preprocess } from './preprocess/index.js';
* @returns {import('#compiler').CompileResult}
*/
export function compile(source, options) {
try {
state.reset({ source, filename: options.filename });
const validated = validate_component_options(options, '');
state.reset(source, validated);

const validated = validate_component_options(options, '');
try {
let parsed = _parse(source);

const { customElement: customElementOptions, ...parsed_options } = parsed.options || {};
Expand All @@ -49,7 +49,7 @@ export function compile(source, options) {
return result;
} catch (e) {
if (e instanceof CompileError) {
handle_compile_error(e, options.filename, source);
handle_compile_error(e);
}

throw e;
Expand All @@ -65,16 +65,16 @@ export function compile(source, options) {
* @returns {import('#compiler').CompileResult}
*/
export function compileModule(source, options) {
try {
state.reset({ source, filename: options.filename });
const validated = validate_module_options(options, '');
state.reset(source, validated);

const validated = validate_module_options(options, '');
try {
const analysis = analyze_module(parse_acorn(source, false), validated);
const result = transform_module(analysis, source, validated);
return result;
} catch (e) {
if (e instanceof CompileError) {
handle_compile_error(e, options.filename, source);
handle_compile_error(e);
}

throw e;
Expand All @@ -83,11 +83,9 @@ export function compileModule(source, options) {

/**
* @param {import('#compiler').CompileError} error
* @param {string | undefined} filename
* @param {string} source
*/
function handle_compile_error(error, filename, source) {
error.filename = filename;
function handle_compile_error(error) {
error.filename = state.filename;

if (error.position) {
const start = state.locator(error.position[0]);
Expand Down Expand Up @@ -134,25 +132,25 @@ function handle_compile_error(error, filename, source) {
*
* https://svelte.dev/docs/svelte-compiler#svelte-parse
* @param {string} source
* @param {{ filename?: string; modern?: boolean }} [options]
* @param {{ filename?: string; rootDir?: string; modern?: boolean }} [options]
* @returns {import('#compiler').Root | import('./types/legacy-nodes.js').LegacyRoot}
*/
export function parse(source, options = {}) {
state.reset({ source, filename: options.filename });
export function parse(source, { filename, rootDir, modern } = {}) {
state.reset(source, { filename, rootDir }); // TODO it's weird to require filename/rootDir here. reconsider the API

/** @type {import('#compiler').Root} */
let ast;
try {
ast = _parse(source);
} catch (e) {
if (e instanceof CompileError) {
handle_compile_error(e, options.filename, source);
handle_compile_error(e);
}

throw e;
}

return to_public_ast(source, ast, options.modern);
return to_public_ast(source, ast, modern);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/src/compiler/migrate/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { migrate_svelte_ignore } from '../utils/extract_svelte_ignore.js';
*/
export function migrate(source) {
try {
reset({ source, filename: 'migrate.svelte' });
reset(source, { filename: 'migrate.svelte' });

let parsed = parse(source);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { javascript_visitors_runes } from './visitors/javascript-runes.js';
import { javascript_visitors_legacy } from './visitors/javascript-legacy.js';
import { serialize_get_binding } from './utils.js';
import { render_stylesheet } from '../css/index.js';
import { filename } from '../../../state.js';

/**
* This function ensures visitor sets don't accidentally clobber each other
Expand Down Expand Up @@ -471,14 +472,7 @@ export function client_component(source, analysis, options) {
}

if (options.dev) {
if (options.filename) {
let filename = options.filename;
if (/(\/|\w:)/.test(options.filename)) {
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved
// filename is absolute — truncate it
const parts = filename.split(/[/\\]/);
filename = parts.length > 3 ? ['...', ...parts.slice(-3)].join('/') : filename;
}

if (filename) {
// add `App.filename = 'App.svelte'` so that we can print useful messages later
body.unshift(
b.stmt(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { DOMBooleanAttributes, HYDRATION_END, HYDRATION_START } from '../../../.
import { escape_html } from '../../../../escaping.js';
import { sanitize_template_string } from '../../../utils/sanitize_template_string.js';
import { BLOCK_CLOSE, BLOCK_CLOSE_ELSE } from '../../../../internal/server/hydration.js';
import { locator } from '../../../state.js';
import { filename, locator } from '../../../state.js';

export const block_open = t_string(`<!--${HYDRATION_START}-->`);
export const block_close = t_string(`<!--${HYDRATION_END}-->`);
Expand Down Expand Up @@ -2398,14 +2398,7 @@ export function server_component(analysis, options) {
body.push(b.export_default(component_function));
}

if (options.dev && options.filename) {
let filename = options.filename;
if (/(\/|\w:)/.test(options.filename)) {
// filename is absolute — truncate it
const parts = filename.split(/[/\\]/);
filename = parts.length > 3 ? ['...', ...parts.slice(-3)].join('/') : filename;
}

if (options.dev && filename) {
// add `App.filename = 'App.svelte'` so that we can print useful messages later
body.unshift(
b.stmt(
Expand Down
22 changes: 15 additions & 7 deletions packages/svelte/src/compiler/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,22 @@ export function pop_ignore() {
}

/**
* @param {{
* source: string;
* filename: string | undefined;
* }} options
* @param {string} source
* @param {{ filename?: string, rootDir?: string }} options
*/
export function reset(options) {
filename = options.filename;
locator = getLocator(options.source, { offsetLine: 1 });
export function reset(source, options) {
if (
typeof options.filename === 'string' &&
typeof options.rootDir === 'string' &&
options.filename.startsWith(options.rootDir)
) {
// make filename relative to rootDir
filename = options.filename.replace(options.rootDir, '').replace(/^[/\\]/, '');
} else {
filename = options.filename;
}

locator = getLocator(source, { offsetLine: 1 });
warnings = [];
ignore_stack = [];
}
9 changes: 8 additions & 1 deletion packages/svelte/src/compiler/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,19 @@ export interface ModuleCompileOptions {
* Used for debugging hints and sourcemaps. Your bundler plugin will set it automatically.
*/
filename?: string;

/**
* Used for ensuring filenames don't leak filesystem information. Your bundler plugin will set it automatically.
* @default process.cwd() on node-like environments, undefined elsewhere
*/
rootDir?: string;
}

// The following two somewhat scary looking types ensure that certain types are required but can be undefined still

export type ValidatedModuleCompileOptions = Omit<Required<ModuleCompileOptions>, 'filename'> & {
export type ValidatedModuleCompileOptions = Omit<Required<ModuleCompileOptions>, 'filename' | 'rootDir'> & {
filename: ModuleCompileOptions['filename'];
rootDir: ModuleCompileOptions['rootDir'];
};

export type ValidatedCompileOptions = ValidatedModuleCompileOptions &
Expand Down
4 changes: 4 additions & 0 deletions packages/svelte/src/compiler/validate-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ import * as w from './warnings.js';
const common = {
filename: string(undefined),

// default to process.cwd() where it exists to replicate svelte4 behavior
// see https://github.com/sveltejs/svelte/blob/b62fc8c8fd2640c9b99168f01b9d958cb2f7574f/packages/svelte/src/compiler/compile/Component.js#L211
rootDir: string(typeof process !== 'undefined' ? process.cwd?.() : undefined),

dev: boolean(false),

generate: validator('client', (input, keypath) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/src/internal/client/dev/elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export function add_locations(fn, filename, locations) {
function assign_location(element, filename, location) {
// @ts-expect-error
element.__svelte_meta = {
loc: { filename, line: location[0], column: location[1] }
loc: { file: filename, line: location[0], column: location[1] }
};

if (location[2]) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export function element(anchor, get_tag, is_svg, render_fn, get_namespace, locat
// @ts-expect-error
element.__svelte_meta = {
loc: {
filename,
file: filename,
line: location[0],
column: location[1]
}
Expand Down
1 change: 1 addition & 0 deletions packages/svelte/tests/runtime-legacy/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ export function runtime_suite(runes: boolean) {
async function common_setup(cwd: string, runes: boolean | undefined, config: RuntimeTest) {
const compileOptions: CompileOptions = {
generate: 'client',
rootDir: cwd,
...config.compileOptions,
immutable: config.immutable,
accessors: 'accessors' in config ? config.accessors : true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ export default test({
},
error:
'bind_invalid_export\n' +
'Component .../export-binding/counter/index.svelte has an export named `increment` that a consumer component is trying to access using `bind:increment`, which is disallowed. Instead, use `bind:this` (e.g. `<Counter bind:this={component} />`) and then access the property on the bound component instance (e.g. `component.increment`)'
'Component counter/index.svelte has an export named `increment` that a consumer component is trying to access using `bind:increment`, which is disallowed. Instead, use `bind:this` (e.g. `<Counter bind:this={component} />`) and then access the property on the bound component instance (e.g. `component.increment`)'
});
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default test({
if (variant === 'hydrate') {
assert.equal(
log[0],
'`<h1>` (.../samples/invalid-html-ssr/Component.svelte:1:0) cannot contain `<p>` (.../samples/invalid-html-ssr/main.svelte:5:0)\n\n' +
'`<h1>` (Component.svelte:1:0) cannot contain `<p>` (main.svelte:5:0)\n\n' +
'This can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.'
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export default test({
assert.htmlEqual(target.innerHTML, `<button>clicks: 1</button>`);

assert.deepEqual(warnings, [
'.../samples/non-local-mutation-discouraged/Counter.svelte mutated a value owned by .../samples/non-local-mutation-discouraged/main.svelte. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead'
'Counter.svelte mutated a value owned by main.svelte. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead'
]);
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ export default test({
},

warnings: [
'.../samples/non-local-mutation-with-binding-2/Intermediate.svelte passed a value to .../samples/non-local-mutation-with-binding-2/Counter.svelte with `bind:`, but the value is owned by .../samples/non-local-mutation-with-binding-2/main.svelte. Consider creating a binding between .../samples/non-local-mutation-with-binding-2/main.svelte and .../samples/non-local-mutation-with-binding-2/Intermediate.svelte'
'Intermediate.svelte passed a value to Counter.svelte with `bind:`, but the value is owned by main.svelte. Consider creating a binding between main.svelte and Intermediate.svelte'
]
});
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default test({
assert.htmlEqual(target.innerHTML, `<button>clicks: 1</button><button>clicks: 1</button>`);

assert.deepEqual(warnings, [
'.../samples/non-local-mutation-with-binding-3/Counter.svelte mutated a value owned by .../samples/non-local-mutation-with-binding-3/main.svelte. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead'
'Counter.svelte mutated a value owned by main.svelte. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead'
]);
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ export default test({

error:
'bind_not_bindable\n' +
'A component is attempting to bind to a non-bindable property `count` belonging to .../samples/props-not-bindable-spread/Counter.svelte (i.e. `<Counter bind:count={...}>`). To mark a property as bindable: `let { count = $bindable() } = $props()`'
'A component is attempting to bind to a non-bindable property `count` belonging to Counter.svelte (i.e. `<Counter bind:count={...}>`). To mark a property as bindable: `let { count = $bindable() } = $props()`'
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ export default test({

error:
'bind_not_bindable\n' +
'A component is attempting to bind to a non-bindable property `count` belonging to .../samples/props-not-bindable/Counter.svelte (i.e. `<Counter bind:count={...}>`). To mark a property as bindable: `let { count = $bindable() } = $props()`'
'A component is attempting to bind to a non-bindable property `count` belonging to Counter.svelte (i.e. `<Counter bind:count={...}>`). To mark a property as bindable: `let { count = $bindable() } = $props()`'
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ export default test({

// @ts-expect-error
assert.deepEqual(ps[0].__svelte_meta.loc, {
filename: '.../samples/svelte-meta-dynamic/main.svelte',
file: 'main.svelte',
line: 7,
column: 0
});

// @ts-expect-error
assert.deepEqual(ps[1].__svelte_meta.loc, {
filename: '.../samples/svelte-meta-dynamic/main.svelte',
file: 'main.svelte',
line: 13,
column: 0
});
Expand All @@ -32,7 +32,7 @@ export default test({

// @ts-expect-error
assert.deepEqual(strong.__svelte_meta.loc, {
filename: '.../samples/svelte-meta-dynamic/main.svelte',
file: 'main.svelte',
line: 10,
column: 1
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ export default test({

// @ts-expect-error
assert.deepEqual(ps[0].__svelte_meta.loc, {
filename: '.../samples/svelte-meta/main.svelte',
file: 'main.svelte',
line: 7,
column: 0
});

// @ts-expect-error
assert.deepEqual(ps[1].__svelte_meta.loc, {
filename: '.../samples/svelte-meta/main.svelte',
file: 'main.svelte',
line: 13,
column: 0
});
Expand All @@ -32,7 +32,7 @@ export default test({

// @ts-expect-error
assert.deepEqual(strong.__svelte_meta.loc, {
filename: '.../samples/svelte-meta/main.svelte',
file: 'main.svelte',
line: 10,
column: 1
});
Expand Down
Loading