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

[fix] resolve $lib alias when packaging #2453

Merged
merged 5 commits into from Sep 21, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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
5 changes: 5 additions & 0 deletions .changeset/rich-seahorses-walk.md
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

Resolve \$lib alias when packaging
54 changes: 48 additions & 6 deletions packages/kit/src/packaging/index.js
Expand Up @@ -14,11 +14,19 @@ const essential_files = ['README', 'LICENSE', 'CHANGELOG', '.gitignore', '.npmig
* @param {string} cwd
*/
export async function make_package(config, cwd = process.cwd()) {
rimraf(path.join(cwd, config.kit.package.dir));
const abs_package_dir = path.join(cwd, config.kit.package.dir);
rimraf(abs_package_dir);

if (config.kit.package.emitTypes) {
// Generate type definitions first so hand-written types can overwrite generated ones
await emit_dts(config);
// Resolve aliases, TS leaves them as-is
const files = walk(abs_package_dir);
for (const file of files) {
const filename = path.join(abs_package_dir, file);
const source = fs.readFileSync(filename, 'utf8');
fs.writeFileSync(filename, resolve_$lib_alias(file, source, config));
}
}

const files_filter = create_filter(config.kit.package.files);
Expand Down Expand Up @@ -47,7 +55,7 @@ export async function make_package(config, cwd = process.cwd()) {

if (!files_filter(file.replace(/\\/g, '/'))) {
const dts_file = (svelte_ext ? file : file.slice(0, -ext.length)) + '.d.ts';
const dts_path = path.join(cwd, config.kit.package.dir, dts_file);
const dts_path = path.join(abs_package_dir, dts_file);
if (fs.existsSync(dts_path)) fs.unlinkSync(dts_path);
continue;
}
Expand All @@ -72,7 +80,7 @@ export async function make_package(config, cwd = process.cwd()) {
// TypeScript's declaration emit won't copy over the d.ts files, so we do it here
out_file = file;
out_contents = source;
if (fs.existsSync(path.join(cwd, config.kit.package.dir, out_file))) {
if (fs.existsSync(path.join(abs_package_dir, out_file))) {
console.warn(
'Found already existing file from d.ts generation for ' +
out_file +
Expand All @@ -86,8 +94,9 @@ export async function make_package(config, cwd = process.cwd()) {
out_file = file;
out_contents = source;
}
out_contents = resolve_$lib_alias(out_file, out_contents, config);

write(path.join(cwd, config.kit.package.dir, out_file), out_contents);
write(path.join(abs_package_dir, out_file), out_contents);

if (exports_filter(file)) {
const original = `$lib/${file.replace(/\\/g, '/')}`;
Expand Down Expand Up @@ -134,7 +143,7 @@ export async function make_package(config, cwd = process.cwd()) {
}
}

write(path.join(cwd, config.kit.package.dir, 'package.json'), JSON.stringify(pkg, null, ' '));
write(path.join(abs_package_dir, 'package.json'), JSON.stringify(pkg, null, ' '));

const whitelist = fs.readdirSync(cwd).filter((file) => {
const lowercased = file.toLowerCase();
Expand All @@ -144,11 +153,44 @@ export async function make_package(config, cwd = process.cwd()) {
const full_path = path.join(cwd, pathname);
if (fs.lstatSync(full_path).isDirectory()) continue; // just to be sure

const package_path = path.join(cwd, config.kit.package.dir, pathname);
const package_path = path.join(abs_package_dir, pathname);
if (!fs.existsSync(package_path)) fs.copyFileSync(full_path, package_path);
}
}

/**
* Resolves the `$lib` alias.
*
* TODO: make this more generic to also handle
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
* other aliases the user could have defined via `kit.vite.resolve.alias`.
*
* @param {string} file Relative to the lib root
* @param {string} content
* @param {import('types/config').ValidatedConfig} config
* @returns {string}
*/
function resolve_$lib_alias(file, content, config) {
/**
* @param {string} match
* @param {string} _
* @param {string} import_path
*/
const replace_import_path = (match, _, import_path) => {
if (!import_path.startsWith('$lib/')) {
return match;
}

const full_path = path.join(config.kit.files.lib, file);
const full_import_path = path.join(config.kit.files.lib, import_path.slice('$lib/'.length));
let resolved = path.relative(path.dirname(full_path), full_import_path).replace(/\\/g, '/');
Copy link
Member

Choose a reason for hiding this comment

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

What happens when full_import_path doesn't actually exists, for whatever reasons it may be (most likely a bad copy-paste or refactor)?

path.relative won't throw as long as it's a string, should we throw an error, leave it with the original alias in place, or use the non-existent resolved path (currently the case)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that's on the user then, the also things like svelte-check should be able find out. We don't check other file paths for correctness, so in my mind it would be inconsistent to check the aliases.

resolved = resolved.startsWith('.') ? resolved : './' + resolved;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe inline this ternary below to keep resolved as const?

return match.replace(import_path, resolved);
};
content = content.replace(/from\s+('|")([^"';,]+?)\1/g, replace_import_path);
content = content.replace(/import\s*\(\s*('|")([^"';,]+?)\1\s*\)/g, replace_import_path);
return content;
}

/**
* @param {string} filename
* @param {string} source
Expand Down
@@ -0,0 +1,4 @@
<script lang="ts">
import { foo } from './sub/foo';
export let bar = foo;
</script>
@@ -0,0 +1,15 @@
import { SvelteComponentTyped } from 'svelte';
declare const __propDef: {
props: {
bar?: import('./sub/foo').Foo;
};
events: {
[evt: string]: CustomEvent<any>;
};
slots: {};
};
export declare type TestProps = typeof __propDef.props;
export declare type TestEvents = typeof __propDef.events;
export declare type TestSlots = typeof __propDef.slots;
export default class Test extends SvelteComponentTyped<TestProps, TestEvents, TestSlots> {}
export {};
@@ -0,0 +1,4 @@
export interface Baz {
baz: string;
}
export declare const baz: Baz;
@@ -0,0 +1 @@
export const baz = { baz: 'baz' };
@@ -0,0 +1 @@
export { default as Test } from './Test.svelte';
@@ -0,0 +1 @@
export { default as Test } from './Test.svelte';
@@ -0,0 +1,15 @@
{
"name": "resolve-alias",
"version": "1.0.0",
"description": "package using $lib alias",
"type": "module",
"exports": {
"./package.json": "./package.json",
"./Test.svelte": "./Test.svelte",
".": "./index.js",
"./baz": "./baz.js",
"./sub/bar": "./sub/bar.js",
"./sub/foo": "./sub/foo.js"
},
"svelte": "./index.js"
}
@@ -0,0 +1,2 @@
export declare const bar1: import('./foo').Foo;
export declare const bar2: import('../baz').Baz;
@@ -0,0 +1,4 @@
import { baz } from '../baz';
import { foo } from './foo';
export const bar1 = foo;
export const bar2 = baz;
@@ -0,0 +1,4 @@
export interface Foo {
foo: string;
}
export declare const foo: Foo;
@@ -0,0 +1 @@
export const foo = { foo: 'foo' };
@@ -0,0 +1,5 @@
{
"name": "resolve-alias",
"version": "1.0.0",
"description": "package using $lib alias"
}
@@ -0,0 +1,12 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8" />
<link rel="icon" href="/favicon.ico" />
<meta name="viewport" content="width=device-width, initial-scale=1" />
%svelte.head%
</head>
<body>
<div id="svelte">%svelte.body%</div>
</body>
</html>
@@ -0,0 +1,5 @@
<script lang="ts">
import { foo } from '$lib/sub/foo';

export let bar = foo;
</script>
@@ -0,0 +1,5 @@
export interface Baz {
baz: string;
}

export const baz: Baz = { baz: 'baz' };
@@ -0,0 +1 @@
export { default as Test } from '$lib/Test.svelte';
@@ -0,0 +1,5 @@
import { baz } from '$lib/baz';
import { foo } from '$lib/sub/foo';

export const bar1 = foo;
export const bar2 = baz;
@@ -0,0 +1,5 @@
export interface Foo {
foo: string;
}

export const foo: Foo = { foo: 'foo' };
@@ -0,0 +1,5 @@
const preprocess = require('svelte-preprocess');

module.exports = {
preprocess: preprocess()
};
@@ -0,0 +1,30 @@
{
"compilerOptions": {
"moduleResolution": "node",
"module": "es2020",
"lib": ["es2020", "DOM"],
"target": "es2019",
/**
svelte-preprocess cannot figure out whether you have a value or a type, so tell TypeScript
to enforce using \`import type\` instead of \`import\` for Types.
*/
"importsNotUsedAsValues": "error",
"isolatedModules": true,
"resolveJsonModule": true,
/**
To have warnings/errors of the Svelte compiler at the correct position,
enable source maps by default.
*/
"sourceMap": true,
"esModuleInterop": true,
"skipLibCheck": true,
"forceConsistentCasingInFileNames": true,
"baseUrl": ".",
"allowJs": true,
"checkJs": true,
"paths": {
"$lib/*": ["src/lib/*"]
}
},
"include": ["src/**/*.d.ts", "src/**/*.js", "src/**/*.ts", "src/**/*.svelte"]
}
4 changes: 4 additions & 0 deletions packages/kit/src/packaging/test/index.js
Expand Up @@ -113,4 +113,8 @@ test('create package with files.exclude settings', async () => {
await test_make_package('files-exclude');
});

test('create package and resolves $lib alias', async () => {
await test_make_package('resolve-alias');
});

test.run();
3 changes: 2 additions & 1 deletion packages/kit/tsconfig.json
Expand Up @@ -16,5 +16,6 @@
"types/*": ["./types/*"]
}
},
"include": ["src/**/*", "test/**/*", "types/**/*"]
"include": ["src/**/*", "test/**/*", "types/**/*"],
"exclude": ["src/packaging/test/fixtures/**/*"]
}