Merge pull request #810 from preactjs/json-import

.changeset/tidy-ways-fold.md

@@ -0,0 +1,5 @@
+---
+'wmr': patch
+---
+
+Fix unable to load json files from outside project public directory.

packages/wmr/src/lib/default-loaders.js

@@ -1,4 +1,5 @@
 import { IMPLICIT_URL } from '../plugins/url-plugin.js';
+import { hasCustomPrefix } from './fs-utils.js';
 import { transformImports } from './transform-imports.js';
 
 /**
@@ -14,14 +15,8 @@ export function defaultLoaders() {
 
 			return await transformImports(code, id, {
 				resolveId(specifier) {
-					const hasPrefix = /^[-\w]+:/.test(specifier);
-
-					if (!hasPrefix) {
-						if (specifier.endsWith('.json')) {
-							return `json:${specifier}`;
-						} else if (IMPLICIT_URL.test(specifier)) {
-							return `url:${specifier}`;
-						}
+					if (!hasCustomPrefix(specifier) && IMPLICIT_URL.test(specifier)) {
+						return `url:${specifier}`;
 					}
 					return null;
 				}

packages/wmr/src/lib/fs-utils.js

@@ -27,3 +27,13 @@ export function isFile(path) {
 		.then(s => s.isFile())
 		.catch(() => false);
 }
+
+/**
+ * Check if an id contains a custom prefix
+ * @param {string} id
+ * @returns {boolean}
+ */
+export function hasCustomPrefix(id) {
+	// Windows disk letters are not prefixes: C:/foo
+	return !/^\0?(?:file|https?):\/\//.test(id) && /^\0?[-\w]{2,}:/.test(id);
+}

packages/wmr/src/plugins/json-plugin.js

@@ -1,9 +1,7 @@
-import path from 'path';
-import { promises as fs } from 'fs';
+import { hasCustomPrefix } from '../lib/fs-utils.js';
 
 /**
- * Convert JSON imports to ESM. Uses a prefix `\0json:./foo.json`.
- * In dev mode, this creates URLs like `/@json/path/to/foo.json`.
+ * Load JSON files
  *
  * @example
  *   import foo from './foo.json';
@@ -15,37 +13,21 @@ import { promises as fs } from 'fs';
  */
 export default function jsonPlugin({ root }) {
 	const IMPORT_PREFIX = 'json:';
-	const INTERNAL_PREFIX = '\0json:';
 
 	return {
 		name: 'json-plugin',
 		async resolveId(id, importer) {
-			if (!id.startsWith(IMPORT_PREFIX)) return;
-
-			id = id.slice(IMPORT_PREFIX.length);
+			if (id.startsWith(IMPORT_PREFIX)) {
+				id = id.slice(IMPORT_PREFIX.length);
+			}
+			if (!id.endsWith('.json')) return;
 
 			const resolved = await this.resolve(id, importer, { skipSelf: true });
-			return resolved && INTERNAL_PREFIX + resolved.id;
+			return resolved && resolved.id;
 		},
-		// TODO: If we find a way to remove the need for adding
-		// an internal prefix we can get rid of the whole
-		// loading logic here and let rollup handle that part.
-		async load(id) {
-			if (!id.startsWith(INTERNAL_PREFIX)) return null;
-
-			id = id.slice(INTERNAL_PREFIX.length);
 
-			// TODO: Add a global helper function to normalize paths
-			// and check that we're allowed to load a file.
-			const file = path.resolve(root, id);
-			if (!file.startsWith(root)) {
-				throw new Error(`JSON file must be placed inside ${root}`);
-			}
-
-			return await fs.readFile(file, 'utf-8');
-		},
 		transform(code, id) {
-			if (!id.startsWith(INTERNAL_PREFIX)) return;
+			if (!id.endsWith('.json') || hasCustomPrefix(id)) return;
 
 			return {
 				code: `export default ${code}`,

packages/wmr/test/fixtures.test.js

@@ -87,13 +87,6 @@ describe('fixtures', () => {
 		});
 	});
 
-	it('should allow overwriting default json loader', async () => {
-		await loadFixture('overwrite-loader-json', env);
-		instance = await runWmrFast(env.tmp.path);
-		const text = await getOutput(env, instance);
-		expect(text).toMatch(/foobarbaz/);
-	});
-
 	it('should allow overwriting default url loader', async () => {
 		await loadFixture('overwrite-loader-url', env);
 		instance = await runWmrFast(env.tmp.path);
@@ -884,22 +877,48 @@ describe('fixtures', () => {
 			await loadFixture('json', env);
 			instance = await runWmrFast(env.tmp.path);
 			await env.page.goto(await instance.address);
-			expect(await env.page.evaluate(`import('/index.js')`)).toEqual({
-				default: {
-					name: 'foo'
-				}
+
+			await withLog(instance.output, async () => {
+				expect(await env.page.evaluate(`import('/index.js')`)).toEqual({
+					default: {
+						name: 'foo'
+					}
+				});
 			});
 		});
 
 		it('should handle "json:" import prefix', async () => {
 			await loadFixture('json', env);
 			instance = await runWmrFast(env.tmp.path);
 			await env.page.goto(await instance.address);
-			expect(await env.page.evaluate(`import('/using-prefix.js')`)).toEqual({
-				default: {
-					second: 'file',
-					a: 42
-				}
+
+			await withLog(instance.output, async () => {
+				expect(await env.page.evaluate(`import('/using-prefix.js')`)).toEqual({
+					default: {
+						second: 'file',
+						a: 42
+					}
+				});
+			});
+		});
+
+		it('should load aliased json files', async () => {
+			await loadFixture('json-alias', env);
+			instance = await runWmrFast(env.tmp.path);
+
+			await withLog(instance.output, async () => {
+				const output = await getOutput(env, instance);
+				expect(output).toMatch(/it works/i);
+			});
+		});
+
+		it('should allow overwriting default json loader', async () => {
+			await loadFixture('overwrite-loader-json', env);
+			instance = await runWmrFast(env.tmp.path);
+
+			await withLog(instance.output, async () => {
+				const text = await getOutput(env, instance);
+				expect(text).toMatch(/foobarbaz/);
 			});
 		});
 	});

packages/wmr/test/fixtures/json-alias/public/index.html

@@ -0,0 +1,2 @@
+<h1>it doesn't work</h1>
+<script type="module" src="index.js"></script>

packages/wmr/test/fixtures/json-alias/public/index.js

@@ -0,0 +1 @@
+import '../src/foo.js';

packages/wmr/test/fixtures/json-alias/src/bar.json

@@ -0,0 +1,3 @@
+{
+	"value": "it works"
+}

packages/wmr/test/fixtures/json-alias/src/foo.js

@@ -0,0 +1,3 @@
+import json from './bar.json';
+
+document.querySelector('h1').textContent = json.value;

packages/wmr/test/fixtures/json-alias/wmr.config.mjs

@@ -0,0 +1,5 @@
+export default {
+	alias: {
+		'src/*': 'src'
+	}
+};

packages/wmr/test/production.test.js

@@ -29,20 +29,20 @@ describe('production', () => {
 	it('should allow overwriting json loader', async () => {
 		await loadFixture('overwrite-loader-json', env);
 		instance = await runWmr(env.tmp.path, 'build');
-		const code = await instance.done;
-		const output = instance.output.join('\n');
-		console.log(output);
 
-		expect(code).toEqual(0);
+		await withLog(instance.output, async () => {
+			const code = await instance.done;
+			expect(code).toEqual(0);
 
-		const { address, stop } = serveStatic(path.join(env.tmp.path, 'dist'));
-		cleanup.push(stop);
+			const { address, stop } = serveStatic(path.join(env.tmp.path, 'dist'));
+			cleanup.push(stop);
 
-		await env.page.goto(address, {
-			waitUntil: ['networkidle0', 'load']
-		});
+			await env.page.goto(address, {
+				waitUntil: ['networkidle0', 'load']
+			});
 
-		expect(await env.page.content()).toMatch(/foobarbaz/);
+			expect(await env.page.content()).toMatch(/foobarbaz/);
+		});
 	});
 
 	it('should throw error on missing module type', async () => {