Skip to content

Run check on all addons test #569

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.

Sign up for GitHub

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Run check on all addons test #569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6218343
updated svelte types in lucia for js with jsdoc
brettearle May 17, 2025
7f1382c
fix: npm run check works except upstream storybook on js output
brettearle May 19, 2025
a1d8bdb
fix: lint
brettearle May 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 71 additions & 55 deletions packages/addons/lucia/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,33 +343,66 @@ export default defineAddon({
return ms.toString();
});

if (typescript) {
sv.file('src/app.d.ts', (content) => {
const { ast, generateCode } = parseScript(content);
sv.file('src/app.d.ts', (content) => {
const { ast, generateCode } = parseScript(content);

const locals = js.kit.addGlobalAppInterface(ast, 'Locals');
if (!locals) {
throw new Error('Failed detecting `locals` interface in `src/app.d.ts`');
}
const locals = js.kit.addGlobalAppInterface(ast, 'Locals');
if (!locals) {
throw new Error('Failed detecting `locals` interface in `src/app.d.ts`');
}

const user = locals.body.body.find((prop) => js.common.hasTypeProp('user', prop));
const session = locals.body.body.find((prop) => js.common.hasTypeProp('session', prop));
const user = locals.body.body.find((prop) => js.common.hasTypeProp('user', prop));
const session = locals.body.body.find((prop) => js.common.hasTypeProp('session', prop));

if (!user) {
locals.body.body.push(createLuciaType('user'));
}
if (!session) {
locals.body.body.push(createLuciaType('session'));
}
return generateCode();
});
}
if (!user) {
locals.body.body.push(createLuciaType('user'));
}
if (!session) {
locals.body.body.push(createLuciaType('session'));
}
return generateCode();
});

sv.file(`src/hooks.server.${ext}`, (content) => {
const { ast, generateCode } = parseScript(content);
js.imports.addNamespace(ast, '$lib/server/auth.js', 'auth');
js.kit.addHooksHandle(ast, typescript, 'handleAuth', getAuthHandleContent());
Copy link
Member

Choose a reason for hiding this comment

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

Can you please elaborate why we are not using this anymore? What was the problem with this call? Is it potentially fixable inside addHooksHandle or do we potentially have to check all other callees of this function for similar issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Flow issue
-> paraglide adds doc comment to solve checking issues with handler types
-> adds to file correctly
-> Lucia parses script (*main issue here = this removes added doc comments as acorn doesn't support them, from my digging **note I've never used acorn)
-> Lucia adds it's doc comment but resulting file misses paraglide.

I went with the string route as the solution here as it also ties into Ben wanting to add more string returns in issue #557.

Copy link
Member

Choose a reason for hiding this comment

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

I'm probably missing something, but I don't see any code adding a comment inside addHooksHandle that could cause this. Also tried running the adapter with JsDoc

Copy link
Contributor Author

@brettearle brettearle Jun 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think it's adding of the comment that fails. I think it is what it holds in memory as the ast (once again it's my first day with acorn and asts). I think it holds everything except the comments. Everytime I tried it with both using ast implementation it would drop 1 of the doc comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will re look into it, I didn't document the why. From memory acorn doesn't read comments back in when it reads the same file back in. So first comment gets dropped when new hook is added, which is why second order has to be strong. But will need to reconfirm my human memory

Copy link
Member

Choose a reason for hiding this comment

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

No problem. Acorn/sv is doing some weirdness around comments, but i did not experience anything like this. Maybe it's just esrap thats not printing them correctly or something.

Also please let me know, once you get back to it, about which comment you are talking exactly, as I didn't manage to figure that out.

return generateCode();
const ms = new MagicString(content);
ms.prepend("import * as auth from '$lib/server/auth.js'\n");

ms.append(
dedent`/** @type {import('@sveltejs/kit').Handle} */
const handleAuth = async ({ event, resolve }) => {
const sessionToken = event.cookies.get(auth.sessionCookieName);

if (!sessionToken) {
event.locals.user = null;
event.locals.session = null;
return resolve(event);
}

const { session, user } = await auth.validateSessionToken(sessionToken);

if (session) {
auth.setSessionTokenCookie(event, sessionToken, session.expiresAt);
} else {
auth.deleteSessionTokenCookie(event);
}

event.locals.user = user;
event.locals.session = session;
return resolve(event);
};
`
);

const handleExportRegex = /^export\s+const\s+handle\s*=\s*([^;]+);?\n?/m;

const match = ms.original.match(handleExportRegex);
if (match) {
ms.prepend("import { sequence } from '@sveltejs/kit/hooks'\n");
const handlerName = match[1]; // e.g. 'handleParaglide'
ms.replace(match[0], '');
ms.append(`\n\nexport const handle = sequence(${handlerName}, handleAuth)`);
}
return ms.toString();
});

if (options.demo) {
Expand All @@ -393,6 +426,8 @@ export default defineAddon({
import * as auth from '$lib/server/auth';
import { db } from '$lib/server/db';
import * as table from '$lib/server/db/schema';

${!typescript ? "/**\n* @type {import('@sveltejs/kit').ServerLoad}\n*/" : ''}
${ts("import type { Actions, PageServerLoad } from './$types';\n")}
export const load${ts(': PageServerLoad')} = async (event) => {
if (event.locals.user) {
Expand All @@ -401,30 +436,31 @@ export default defineAddon({
return {};
};

${!typescript ? "/**\n* @type {import('@sveltejs/kit').Actions}\n*/" : ''}
export const actions${ts(': Actions')} = {
login: async (event) => {
const formData = await event.request.formData();
const username = formData.get('username');
const password = formData.get('password');

if (!validateUsername(username)) {
if (!username || !validateUsername(username)) {
return fail(400, { message: 'Invalid username (min 3, max 31 characters, alphanumeric only)' });
}
if (!validatePassword(password)) {
if (!password || !validatePassword(password)) {
return fail(400, { message: 'Invalid password (min 6, max 255 characters)' });
}

const results = await db
.select()
.from(table.user)
.where(eq(table.user.username, username));
.where(eq(table.user.username, username.toString()));

const existingUser = results.at(0);
if (!existingUser) {
return fail(400, { message: 'Incorrect username or password' });
}

const validPassword = await verify(existingUser.passwordHash, password, {
const validPassword = await verify(existingUser.passwordHash, password.toString(), {
memoryCost: 19456,
timeCost: 2,
outputLen: 32,
Expand All @@ -445,15 +481,15 @@ export default defineAddon({
const username = formData.get('username');
const password = formData.get('password');

if (!validateUsername(username)) {
if (!username || !validateUsername(username)) {
return fail(400, { message: 'Invalid username' });
}
if (!validatePassword(password)) {
if (!password || !validatePassword(password)) {
return fail(400, { message: 'Invalid password' });
}

const userId = generateUserId();
const passwordHash = await hash(password, {
const passwordHash = await hash(password.toString(), {
// recommended minimum parameters
memoryCost: 19456,
timeCost: 2,
Expand All @@ -462,7 +498,7 @@ export default defineAddon({
});

try {
await db.insert(table.user).values({ id: userId, username, passwordHash });
await db.insert(table.user).values({ id: userId, username: username.toString(), passwordHash });

const sessionToken = auth.generateSessionToken();
const session = await auth.createSession(sessionToken, userId);
Expand All @@ -481,6 +517,7 @@ export default defineAddon({
return id;
}

${!typescript ? '/** @param {unknown} username */' : ''}
function validateUsername(username${ts(': unknown')})${ts(': username is string')} {
return (
typeof username === 'string' &&
Expand All @@ -490,6 +527,7 @@ export default defineAddon({
);
}

${!typescript ? '/** @param {unknown} password */' : ''}
function validatePassword(password${ts(': unknown')})${ts(': password is string')} {
return (
typeof password === 'string' &&
Expand Down Expand Up @@ -556,18 +594,20 @@ export default defineAddon({
log.warn(`Existing ${colors.yellow(filePath)} file. Could not update.`);
return content;
}

const [ts] = utils.createPrinter(typescript);
return dedent`
import * as auth from '$lib/server/auth';
import { fail, redirect } from '@sveltejs/kit';
import { getRequestEvent } from '$app/server';

${!typescript ? "/**\n* @type {import('@sveltejs/kit').ServerLoad}\n*/" : ''}
${ts("import type { Actions, PageServerLoad } from './$types';\n")}
export const load${ts(': PageServerLoad')} = async () => {
const user = requireLogin()
return { user };
};

${!typescript ? "/**\n* @type {import('@sveltejs/kit').Actions}\n*/" : ''}
export const actions${ts(': Actions')} = {
logout: async (event) => {
if (!event.locals.session) {
Expand Down Expand Up @@ -661,30 +701,6 @@ function createLuciaType(name: string): AstTypes.TSInterfaceBody['body'][number]
};
}

function getAuthHandleContent() {
return `
async ({ event, resolve }) => {
const sessionToken = event.cookies.get(auth.sessionCookieName);
if (!sessionToken) {
event.locals.user = null;
event.locals.session = null;
return resolve(event);
}

const { session, user } = await auth.validateSessionToken(sessionToken);
if (session) {
auth.setSessionTokenCookie(event, sessionToken, session.expiresAt);
} else {
auth.deleteSessionTokenCookie(event);
}

event.locals.user = user;
event.locals.session = session;

return resolve(event);
};`;
}

function getCallExpression(ast: AstTypes.Node): AstTypes.CallExpression | undefined {
let callExpression;

Expand Down
34 changes: 28 additions & 6 deletions packages/addons/paraglide/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import {
object,
variables,
exports,
kit as kitJs
kit as kitJs,
type AstTypes
} from '@sveltejs/cli-core/js';
import * as html from '@sveltejs/cli-core/html';
import { parseHtml, parseJson, parseScript, parseSvelte } from '@sveltejs/cli-core/parsers';
import { addToDemoPage } from '../common.ts';
import { addJsDocTypeComment } from '../../core/tooling/js/common.ts';

const DEFAULT_INLANG_PROJECT = {
$schema: 'https://inlang.com/schema/project-settings',
Expand Down Expand Up @@ -139,13 +141,33 @@ export default defineAddon({
});

const hookHandleContent = `({ event, resolve }) => paraglideMiddleware(event.request, ({ request, locale }) => {
event.request = request;
return resolve(event, {
transformPageChunk: ({ html }) => html.replace('%paraglide.lang%', locale)
});
});`;
event.request = request;
return resolve(event, {
transformPageChunk: ({ html }) => html.replace('%paraglide.lang%', locale)
});
});`;

kitJs.addHooksHandle(ast, typescript, 'handleParaglide', hookHandleContent);

const hookDecl = findVariableDeclaration(ast, 'handleParaglide');

if (hookDecl) {
addJsDocTypeComment(hookDecl, "import('@sveltejs/kit').Handle");
}
function findVariableDeclaration(
ast: AstTypes.Program,
name: string
): AstTypes.VariableDeclaration | undefined {
return ast.body.find(
(n): n is AstTypes.VariableDeclaration =>
n.type === 'VariableDeclaration' &&
n.declarations.some(
(d) =>
d.type === 'VariableDeclarator' && d.id.type === 'Identifier' && d.id.name === name
)
);
}

return generateCode();
});

Expand Down
3 changes: 2 additions & 1 deletion packages/core/tooling/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,9 @@ export function parseScript(content: string): TsEstree.Program {
const indentation = content.slice(a, b);
value = value.replace(new RegExp(`^${indentation}`, 'gm'), '');
}
const raw = content.slice(start, end);

comments.push({ type: block ? 'Block' : 'Line', value, start, end });
comments.push({ type: block ? 'Block' : 'Line', value: block ? raw : value, start, end });
}
}) as TsEstree.Program;

Expand Down