Skip to content

Commit 0e41a90

Browse files
samuelstroscheinsamuelstroschein
authored
Merge pull request #3542 from opral/parjs-490-plugin-import-errors-are-silently-ignored
Parjs-490-plugin-import-errors-are-silently-ignored
2 parents 5f932e2 + 02c2d34 commit 0e41a90

File tree

9 files changed

+233
-58
lines changed

9 files changed

+233
-58
lines changed

‎.changeset/fast-pianos-roll.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@inlang/paraglide-js": patch
3+
---
4+
5+
improve: compiler should log warnings when plugins can not be imported

‎.changeset/ninety-baboons-burn.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@inlang/sdk": patch
3+
---
4+
5+
fix: `loadProjectFromDirectory()` should return errors from `loadProject()`

‎inlang/packages/paraglide/paraglide-js/src/compiler/compile.test.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { compile } from "./compile.js";
99
import { getAccountFilePath } from "../services/account/index.js";
1010
import type { Runtime } from "./runtime/type.js";
1111
import { defaultCompilerOptions } from "./compiler-options.js";
12+
import consola from "consola";
1213

1314
test("loads a project and compiles it", async () => {
1415
const project = await loadProjectInMemory({
@@ -338,3 +339,69 @@ test("default compiler options should include cookied, variable and baseLocale t
338339
"baseLocale",
339340
]);
340341
});
342+
343+
test("emits warnings for modules that couldn't be imported locally", async () => {
344+
const project = await loadProjectInMemory({
345+
blob: await newProject({
346+
settings: {
347+
baseLocale: "en",
348+
locales: ["en", "de", "fr"],
349+
modules: ["./non-existent-paraglide-plugin.js"],
350+
},
351+
}),
352+
});
353+
354+
const mock = vi.fn();
355+
356+
consola.mockTypes(() => mock);
357+
358+
const fs = memfs().fs as unknown as typeof import("node:fs");
359+
360+
// save project to directory to test loading
361+
await saveProjectToDirectory({
362+
project,
363+
path: "/project.inlang",
364+
fs: fs.promises,
365+
});
366+
367+
await compile({
368+
project: "/project.inlang",
369+
outdir: "/output",
370+
fs: fs,
371+
});
372+
373+
expect(mock).toHaveBeenCalled();
374+
});
375+
376+
test("emits warnings for modules that couldn't be imported via http", async () => {
377+
const project = await loadProjectInMemory({
378+
blob: await newProject({
379+
settings: {
380+
baseLocale: "en",
381+
locales: ["en", "de", "fr"],
382+
modules: ["https://example.com/non-existent-paraglide-plugin.js"],
383+
},
384+
}),
385+
});
386+
387+
const mock = vi.fn();
388+
389+
consola.mockTypes(() => mock);
390+
391+
const fs = memfs().fs as unknown as typeof import("node:fs");
392+
393+
// save project to directory to test loading
394+
await saveProjectToDirectory({
395+
project,
396+
path: "/project.inlang",
397+
fs: fs.promises,
398+
});
399+
400+
await compile({
401+
project: "/project.inlang",
402+
outdir: "/output",
403+
fs: fs,
404+
});
405+
406+
expect(mock).toHaveBeenCalled();
407+
});

‎inlang/packages/paraglide/paraglide-js/src/compiler/compile.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
defaultCompilerOptions,
1212
type CompilerOptions,
1313
} from "./compiler-options.js";
14+
import { Logger } from "../services/logger/index.js";
1415

1516
// This is a workaround to prevent multiple compilations from running at the same time.
1617
// https://github.com/opral/inlang-paraglide-js/issues/320#issuecomment-2596951222
@@ -22,6 +23,8 @@ export type CompilationResult = {
2223
outputHashes: Record<string, string> | undefined;
2324
};
2425

26+
const logger = new Logger();
27+
2528
/**
2629
* Loads, compiles, and writes the output to disk.
2730
*
@@ -88,6 +91,12 @@ export async function compile(
8891
saveLocalAccount({ fs, account: activeAccount });
8992
}
9093

94+
const warningsAndErrors = await project.errors.get();
95+
96+
for (const warningOrError of warningsAndErrors) {
97+
logger.warn(warningOrError);
98+
}
99+
91100
await project.close();
92101

93102
return { outputHashes };

‎inlang/packages/paraglide/paraglide-js/src/compiler/output-structure/locale-modules.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,10 @@ test("should handle case sensitivity in message IDs correctly", () => {
115115
const fallbackMap: Record<string, string | undefined> = {};
116116

117117
const output = generateOutput(bundles, settings, fallbackMap);
118-
118+
119119
// Check that the output exists
120120
expect(output).toHaveProperty("messages/en.js");
121-
121+
122122
// The exported constants should not conflict
123123
const content = output["messages/en.js"];
124124
expect(content).toContain("export const sad_penguin_bundle");

‎inlang/packages/paraglide/paraglide-js/src/compiler/output-structure/locale-modules.ts‎

Lines changed: 60 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { inputsType } from "../jsdoc-types.js";
55

66
// Helper function to escape special characters in a string for use in a regular expression
77
function escapeRegExp(string: string) {
8-
return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
8+
return string.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
99
}
1010

1111
// This map will be used to track which bundle IDs have been renamed to which unique IDs
@@ -15,13 +15,13 @@ const bundleIdToUniqueIdMap = new Map<string, string>();
1515
export function messageReferenceExpression(locale: string, bundleId: string) {
1616
// First convert to safe module ID
1717
const safeModuleId = toSafeModuleId(bundleId);
18-
18+
1919
// Check if this bundleId has been mapped to a unique identifier
2020
const uniqueId = bundleIdToUniqueIdMap.get(bundleId);
2121
if (uniqueId) {
2222
return `${toSafeModuleId(locale)}.${uniqueId}`;
2323
}
24-
24+
2525
// Otherwise, return the default safe module ID
2626
return `${toSafeModuleId(locale)}.${safeModuleId}`;
2727
}
@@ -33,55 +33,61 @@ export function generateOutput(
3333
): Record<string, string> {
3434
// Create a map to track module IDs in the index file to avoid duplicates
3535
const indexModuleIdMap = new Map<string, string>();
36-
36+
3737
// Process the bundles to ensure no duplicate bundle IDs
3838
// Generate unique moduleIds for duplicate IDs
39-
const processedBundleCodes = compiledBundles.map(({ bundle }) => {
40-
const bundleId = bundle.node.id;
41-
const bundleModuleId = toSafeModuleId(bundleId);
42-
43-
// Check if this safe module ID has been used before
44-
if (indexModuleIdMap.has(bundleModuleId)) {
45-
// Create a unique identifier by adding a counter
46-
let counter = 1;
47-
let uniqueModuleId = `${bundleModuleId}${counter}`;
48-
49-
while (indexModuleIdMap.has(uniqueModuleId)) {
50-
counter++;
51-
uniqueModuleId = `${bundleModuleId}${counter}`;
39+
const processedBundleCodes = compiledBundles
40+
.map(({ bundle }) => {
41+
const bundleId = bundle.node.id;
42+
const bundleModuleId = toSafeModuleId(bundleId);
43+
44+
// Check if this safe module ID has been used before
45+
if (indexModuleIdMap.has(bundleModuleId)) {
46+
// Create a unique identifier by adding a counter
47+
let counter = 1;
48+
let uniqueModuleId = `${bundleModuleId}${counter}`;
49+
50+
while (indexModuleIdMap.has(uniqueModuleId)) {
51+
counter++;
52+
uniqueModuleId = `${bundleModuleId}${counter}`;
53+
}
54+
55+
// Modify the code to use the unique identifier
56+
const modifiedCode = bundle.code
57+
.replace(
58+
new RegExp(`const ${bundleModuleId} =`, "g"),
59+
`const ${uniqueModuleId} =`
60+
)
61+
.replace(
62+
new RegExp(`export const ${bundleModuleId} =`, "g"),
63+
`export const ${uniqueModuleId} =`
64+
)
65+
.replace(
66+
new RegExp(`export { ${bundleModuleId}`, "g"),
67+
`export { ${uniqueModuleId}`
68+
)
69+
.replace(
70+
// Also update the trackMessageCall to use the new identifier
71+
new RegExp(`trackMessageCall\\("${escapeRegExp(bundleId)}"`, "g"),
72+
`trackMessageCall("${bundleId}"`
73+
);
74+
75+
// Store the unique ID mapping
76+
indexModuleIdMap.set(uniqueModuleId, bundleId);
77+
78+
// Also store in the global map for messageReferenceExpression to use
79+
bundleIdToUniqueIdMap.set(bundleId, uniqueModuleId);
80+
81+
return modifiedCode;
5282
}
53-
54-
// Modify the code to use the unique identifier
55-
const modifiedCode = bundle.code.replace(
56-
new RegExp(`const ${bundleModuleId} =`, 'g'),
57-
`const ${uniqueModuleId} =`
58-
).replace(
59-
new RegExp(`export const ${bundleModuleId} =`, 'g'),
60-
`export const ${uniqueModuleId} =`
61-
).replace(
62-
new RegExp(`export { ${bundleModuleId}`, 'g'),
63-
`export { ${uniqueModuleId}`
64-
).replace(
65-
// Also update the trackMessageCall to use the new identifier
66-
new RegExp(`trackMessageCall\\("${escapeRegExp(bundleId)}"`, 'g'),
67-
`trackMessageCall("${bundleId}"`
68-
);
69-
70-
// Store the unique ID mapping
71-
indexModuleIdMap.set(uniqueModuleId, bundleId);
72-
73-
// Also store in the global map for messageReferenceExpression to use
74-
bundleIdToUniqueIdMap.set(bundleId, uniqueModuleId);
75-
76-
return modifiedCode;
77-
}
78-
79-
// Store the mapping
80-
indexModuleIdMap.set(bundleModuleId, bundleId);
81-
82-
return bundle.code;
83-
}).join("\n");
84-
83+
84+
// Store the mapping
85+
indexModuleIdMap.set(bundleModuleId, bundleId);
86+
87+
return bundle.code;
88+
})
89+
.join("\n");
90+
8591
const indexFile = [
8692
`import { getLocale, trackMessageCall, experimentalMiddlewareLocaleSplitting, isServer } from "../runtime.js"`,
8793
settings.locales
@@ -101,15 +107,15 @@ export function generateOutput(
101107
for (const locale of settings.locales) {
102108
const filename = `messages/${locale}.js`;
103109
let file = "";
104-
110+
105111
// Keep track of module IDs to avoid duplicates
106112
const moduleIdMap = new Map<string, string>();
107113

108114
for (const compiledBundle of compiledBundles) {
109115
const compiledMessage = compiledBundle.messages[locale];
110116
const bundleId = compiledBundle.bundle.node.id;
111117
const bundleModuleId = toSafeModuleId(compiledBundle.bundle.node.id);
112-
118+
113119
// Check if this module ID has already been used
114120
let uniqueModuleId = bundleModuleId;
115121
if (moduleIdMap.has(bundleModuleId)) {
@@ -121,10 +127,10 @@ export function generateOutput(
121127
uniqueModuleId = `${bundleModuleId}${counter}`;
122128
}
123129
}
124-
130+
125131
// Store this module ID
126132
moduleIdMap.set(uniqueModuleId, bundleId);
127-
133+
128134
const inputs =
129135
compiledBundle.bundle.node.declarations?.filter(
130136
(decl) => decl.type === "input-variable"
@@ -152,4 +158,4 @@ export function generateOutput(
152158
output[filename] = file;
153159
}
154160
return output;
155-
}
161+
}

‎inlang/packages/sdk/src/plugin/importPlugins.test.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,29 @@ test("if a fetch fails, a plugin import error is expected", async () => {
4949
expect(result.errors[0]).toBeInstanceOf(PluginImportError);
5050
});
5151

52+
test("if a network error occurs during fetch, a plugin import error is expected", async () => {
53+
global.fetch = vi.fn().mockRejectedValue(new Error("Network error"));
54+
const lix = await openLixInMemory({ blob: await newLixFile() });
55+
56+
const result = await importPlugins({
57+
lix,
58+
settings: {
59+
baseLocale: "en",
60+
locales: ["en"],
61+
modules: ["https://example.com/non-existent-paraglide-plugin.js"],
62+
},
63+
});
64+
65+
expect(global.fetch).toHaveBeenCalledTimes(1);
66+
expect(result.plugins.length).toBe(0);
67+
expect(result.errors.length).toBe(1);
68+
expect(result.errors[0]).toBeInstanceOf(PluginImportError);
69+
expect(result.errors[0]?.message).toContain(
70+
"https://example.com/non-existent-paraglide-plugin.js"
71+
);
72+
expect(result.errors[0]?.message).toContain("Network error");
73+
});
74+
5275
test("it should filter message lint rules for legacy reasons", async () => {
5376
global.fetch = vi.fn().mockResolvedValue({
5477
ok: true,

‎inlang/packages/sdk/src/project/loadProject.test.ts‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import { expect, test } from "vitest";
1+
import { expect, test, vi } from "vitest";
22
import { newProject } from "./newProject.js";
33
import { loadProjectInMemory } from "./loadProjectInMemory.js";
4+
import { PluginImportError } from "../plugin/errors.js";
45
import { validate } from "uuid";
56

67
test("it should persist changes of bundles, messages, and variants to lix ", async () => {
@@ -174,3 +175,40 @@ test("closing a project should not lead to a throw", async () => {
174175
// capture async throws
175176
await new Promise((resolve) => setTimeout(resolve, 250));
176177
});
178+
179+
test("project.errors.get() returns errors for modules that couldn't be imported via http", async () => {
180+
// Mock global fetch to simulate a network error
181+
global.fetch = vi.fn().mockRejectedValue(new Error("Network error"));
182+
183+
const project = await loadProjectInMemory({
184+
blob: await newProject({
185+
settings: {
186+
baseLocale: "en",
187+
locales: ["en", "de", "fr"],
188+
modules: ["https://example.com/non-existent-paraglide-plugin.js"],
189+
},
190+
}),
191+
});
192+
193+
// Get the errors from project
194+
const errors = await project.errors.get();
195+
196+
// Verify there's at least one error
197+
expect(errors.length).toBeGreaterThan(0);
198+
199+
// Find the error related to the HTTP import
200+
const httpImportError = errors.find(
201+
(error) =>
202+
error instanceof PluginImportError &&
203+
error.plugin === "https://example.com/non-existent-paraglide-plugin.js"
204+
);
205+
206+
// Verify the error exists and contains appropriate information
207+
expect(httpImportError).toBeDefined();
208+
expect(httpImportError?.message).toContain(
209+
"non-existent-paraglide-plugin.js"
210+
);
211+
expect(httpImportError?.message).toContain("Couldn't import");
212+
213+
await project.close();
214+
});

0 commit comments

Comments
 (0)