From f32f59978c7b10dbe30355da1e4b4d917c7cd1d4 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 3 Dec 2020 12:12:08 -0500 Subject: [PATCH 1/4] transform exported functions correctly - fixes #224 --- packages/kit/src/api/dev/loader.js | 144 ++------------------- packages/kit/src/api/dev/transform.js | 137 ++++++++++++++++++++ packages/kit/src/api/dev/transform.spec.js | 58 +++++++++ 3 files changed, 205 insertions(+), 134 deletions(-) create mode 100644 packages/kit/src/api/dev/transform.js create mode 100644 packages/kit/src/api/dev/transform.spec.js diff --git a/packages/kit/src/api/dev/loader.js b/packages/kit/src/api/dev/loader.js index 7dc136a8a21d..210bcd70f08f 100644 --- a/packages/kit/src/api/dev/loader.js +++ b/packages/kit/src/api/dev/loader.js @@ -1,9 +1,6 @@ import { URL } from 'url'; import { resolve, relative } from 'path'; -import * as meriyah from 'meriyah'; -import MagicString from 'magic-string'; -import { extract_names } from 'periscopic'; -import { walk } from 'estree-walker'; +import { transform } from './transform'; // This function makes it possible to load modules from the 'server' // snowpack server, for the sake of SSR @@ -47,7 +44,6 @@ export default function loader(snowpack, config) { }); async function load(url, url_stack) { - // TODO: meriyah (JS parser) doesn't support `import.meta.hot = ...` used in HMR setup code. if (url.endsWith('.css.proxy.js')) { return null; } @@ -75,133 +71,7 @@ export default function loader(snowpack, config) { } async function initialize_module(url, data, url_stack) { - const code = new MagicString(data); - const ast = meriyah.parseModule(data, { - ranges: true, - next: true - }); - - const imports = []; - - const export_from_identifiers = new Map(); - let uid = 1; - - ast.body.forEach((node) => { - if (node.type === 'ImportDeclaration') { - imports.push(node); - code.remove(node.start, node.end); - } - - if (node.type === 'ExportAllDeclaration') { - if (!export_from_identifiers.has(node.source)) { - export_from_identifiers.set(node.source, `__import${uid++}`); - } - - code.overwrite( - node.start, - node.end, - `Object.assign(exports, ${export_from_identifiers.get(node.source)})` - ); - imports.push(node); - } - - if (node.type === 'ExportDefaultDeclaration') { - code.overwrite(node.start, node.declaration.start, 'exports.default = '); - } - - if (node.type === 'ExportNamedDeclaration') { - if (node.source) { - imports.push(node); - - if (!export_from_identifiers.has(node.source)) { - export_from_identifiers.set(node.source, `__import${uid++}`); - } - } - - if (node.specifiers && node.specifiers.length > 0) { - code.remove(node.start, node.specifiers[0].start); - - node.specifiers.forEach((specifier) => { - const lhs = `exports.${specifier.exported.name}`; - const rhs = node.source - ? `${export_from_identifiers.get(node.source)}.${specifier.local.name}` - : specifier.local.name; - - code.overwrite(specifier.start, specifier.end, `${lhs} = ${rhs}`); - }); - - code.remove(node.specifiers[node.specifiers.length - 1].end, node.end); - } else { - // `export const foo = ...` or `export function foo() {...}` - if (node.declaration.type === 'VariableDeclaration') { - code.remove(node.start, node.declaration.start); - - const names = []; - node.declaration.declarations.forEach((declarator) => { - names.push(...extract_names(declarator.id)); - }); - - code.appendLeft(node.end, names.map((name) => ` exports.${name} = ${name};`).join('')); - } else { - code.overwrite( - node.start, - node.declaration.start, - `exports.${node.declaration.id.name} = ` - ); - } - } - } - }); - - // replace import.meta and import(dynamic) - if (/import\s*\.\s*meta/.test(data) || /import\s*\(/.test(data)) { - walk(ast.body, { - enter(node) { - if (node.type === 'MetaProperty' && node.meta.name === 'import') { - code.overwrite(node.start, node.end, '__importmeta__'); - } else if (node.type === 'ImportExpression') { - code.overwrite(node.start, node.start + 6, '__import__'); - } - } - }); - } - - const deps = []; - imports.forEach((node) => { - const promise = get_module(url, node.source.value, url_stack); - - if (node.type === 'ExportAllDeclaration' || node.type === 'ExportNamedDeclaration') { - // `export * from './other.js'` or `export { foo } from './other.js'` - deps.push({ - name: export_from_identifiers.get(node.source), - promise - }); - } else if (node.specifiers.length === 0) { - // bare import - deps.push({ - name: null, - promise - }); - } else if (node.specifiers[0].type === 'ImportNamespaceSpecifier') { - deps.push({ - name: node.specifiers[0].local.name, - promise - }); - } else { - deps.push( - ...node.specifiers.map((specifier) => ({ - name: specifier.local.name, - promise: promise.then( - (exports) => exports[specifier.imported ? specifier.imported.name : 'default'] - ) - })) - ); - } - }); - - deps.sort((a, b) => (!!a.name !== !!b.name ? (a.name ? -1 : 1) : 0)); - - code.append(`\n//# sourceURL=${url}`); + const { code, deps } = transform(url, data); const fn = new Function( 'exports', @@ -210,9 +80,15 @@ export default function loader(snowpack, config) { '__import__', '__importmeta__', ...deps.map((d) => d.name).filter(Boolean), - code.toString() + `${code}\n//# sourceURL=${url}` ); - const values = await Promise.all(deps.map((d) => d.promise)); + + const promises = deps.map(async (dep) => { + const mod = await get_module(url, dep.source); + return dep.prop ? mod[dep.prop] : dep; + }); + + const values = await Promise.all(promises); const exports = {}; diff --git a/packages/kit/src/api/dev/transform.js b/packages/kit/src/api/dev/transform.js new file mode 100644 index 000000000000..c96e43d5e8fd --- /dev/null +++ b/packages/kit/src/api/dev/transform.js @@ -0,0 +1,137 @@ +import * as meriyah from 'meriyah'; +import MagicString from 'magic-string'; +import { extract_names } from 'periscopic'; +import { walk } from 'estree-walker'; + +export function transform(data) { + const code = new MagicString(data); + const ast = meriyah.parseModule(data, { + ranges: true, + next: true + }); + + const imports = []; + + const export_from_identifiers = new Map(); + let uid = 1; + + ast.body.forEach((node) => { + if (node.type === 'ImportDeclaration') { + imports.push(node); + code.remove(node.start, node.end); + } + + if (node.type === 'ExportAllDeclaration') { + if (!export_from_identifiers.has(node.source)) { + export_from_identifiers.set(node.source, `__import${uid++}`); + } + + code.overwrite( + node.start, + node.end, + `Object.assign(exports, ${export_from_identifiers.get(node.source)})` + ); + imports.push(node); + } + + if (node.type === 'ExportDefaultDeclaration') { + code.overwrite(node.start, node.declaration.start, 'exports.default = '); + } + + if (node.type === 'ExportNamedDeclaration') { + if (node.source) { + imports.push(node); + + if (!export_from_identifiers.has(node.source)) { + export_from_identifiers.set(node.source, `__import${uid++}`); + } + } + + if (node.specifiers && node.specifiers.length > 0) { + code.remove(node.start, node.specifiers[0].start); + + node.specifiers.forEach((specifier) => { + const lhs = `exports.${specifier.exported.name}`; + const rhs = node.source + ? `${export_from_identifiers.get(node.source)}.${specifier.local.name}` + : specifier.local.name; + + code.overwrite(specifier.start, specifier.end, `${lhs} = ${rhs}`); + }); + + code.remove(node.specifiers[node.specifiers.length - 1].end, node.end); + } else { + // `export const foo = ...` or `export function foo() {...}` + code.remove(node.start, node.declaration.start); + + let suffix; + + if (node.declaration.type === 'VariableDeclaration') { + const names = []; + node.declaration.declarations.forEach((declarator) => { + names.push(...extract_names(declarator.id)); + }); + + suffix = names.map((name) => ` exports.${name} = ${name};`).join(''); + } else { + const { name } = node.declaration.id; + suffix = ` exports.${name} = ${name};`; + } + + code.appendLeft(node.end, suffix); + } + } + }); + + // replace import.meta and import(dynamic) + if (/import\s*\.\s*meta/.test(data) || /import\s*\(/.test(data)) { + walk(ast.body, { + enter(node) { + if (node.type === 'MetaProperty' && node.meta.name === 'import') { + code.overwrite(node.start, node.end, '__importmeta__'); + } else if (node.type === 'ImportExpression') { + code.overwrite(node.start, node.start + 6, '__import__'); + } + } + }); + } + + const deps = []; + imports.forEach((node) => { + const source = node.source.value; + + if (node.type === 'ExportAllDeclaration' || node.type === 'ExportNamedDeclaration') { + // `export * from './other.js'` or `export { foo } from './other.js'` + deps.push({ + name: export_from_identifiers.get(node.source), + prop: null, + source + }); + } else if (node.specifiers.length === 0) { + // bare import + deps.push({ + name: null, + prop: null, + source + }); + } else if (node.specifiers[0].type === 'ImportNamespaceSpecifier') { + deps.push({ + name: node.specifiers[0].local.name, + prop: null, + source + }); + } else { + deps.push( + ...node.specifiers.map((specifier) => ({ + name: specifier.local.name, + prop: specifier.imported ? specifier.imported.name : 'default', + source + })) + ); + } + }); + + deps.sort((a, b) => (!!a.name !== !!b.name ? (a.name ? -1 : 1) : 0)); + + return { code: code.toString(), deps }; +} diff --git a/packages/kit/src/api/dev/transform.spec.js b/packages/kit/src/api/dev/transform.spec.js new file mode 100644 index 000000000000..9f71c2c803af --- /dev/null +++ b/packages/kit/src/api/dev/transform.spec.js @@ -0,0 +1,58 @@ +import { test } from 'uvu'; +import * as assert from 'uvu/assert'; +import { transform } from './transform'; + +function compare(a, b) { + assert.equal( + a.replace(/^\t+/gm, '').trim(), + b.replace(/^\t+/gm, '').trim(), + ); +} + +test('extracts imports', () => { + const { code, deps } = transform(` + import './empty.js'; + import foo from './foo.js'; + import { bar } from './bar.js'; + import * as baz from './baz.js'; + + console.log(foo, bar, baz); + `); + + compare(code, ` + console.log(foo, bar, baz); + `); + + assert.equal(deps, [ + { name: 'foo', prop: 'default', source: './foo.js' }, + { name: 'bar', prop: 'bar', source: './bar.js' }, + { name: 'baz', prop: null, source: './baz.js' }, + { name: null, prop: null, source: './empty.js' } + ]); +}); + +test('transforms exported functions safely', () => { + const { code, deps } = transform(` + export function foo() { + console.log('foo'); + } + + export function bar() { + foo(); + } + `); + + compare(code, ` + function foo() { + console.log('foo'); + } exports.foo = foo; + + function bar() { + foo(); + } exports.bar = bar; + `); + + assert.equal(deps, []); +}); + +test.run(); From bc8562c7a8ea11863641c1f2b456e54b4c36c771 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 3 Dec 2020 12:12:46 -0500 Subject: [PATCH 2/4] add changeset --- .changeset/tasty-donkeys-wait.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tasty-donkeys-wait.md diff --git a/.changeset/tasty-donkeys-wait.md b/.changeset/tasty-donkeys-wait.md new file mode 100644 index 000000000000..b1e8f9c76395 --- /dev/null +++ b/.changeset/tasty-donkeys-wait.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +Transform exported functions correctly From 41baacf0f806ad44daa921caad002b2653cdffe5 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 3 Dec 2020 12:16:00 -0500 Subject: [PATCH 3/4] lint --- packages/kit/src/api/dev/transform.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/api/dev/transform.spec.js b/packages/kit/src/api/dev/transform.spec.js index 9f71c2c803af..c45daa486326 100644 --- a/packages/kit/src/api/dev/transform.spec.js +++ b/packages/kit/src/api/dev/transform.spec.js @@ -5,7 +5,7 @@ import { transform } from './transform'; function compare(a, b) { assert.equal( a.replace(/^\t+/gm, '').trim(), - b.replace(/^\t+/gm, '').trim(), + b.replace(/^\t+/gm, '').trim() ); } From 05d93f053345e446dd119070723f26177267226b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 3 Dec 2020 12:35:58 -0500 Subject: [PATCH 4/4] fix daft mistakes --- packages/kit/src/api/dev/loader.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/kit/src/api/dev/loader.js b/packages/kit/src/api/dev/loader.js index 210bcd70f08f..ddf1bc9a8f3b 100644 --- a/packages/kit/src/api/dev/loader.js +++ b/packages/kit/src/api/dev/loader.js @@ -71,7 +71,7 @@ export default function loader(snowpack, config) { } async function initialize_module(url, data, url_stack) { - const { code, deps } = transform(url, data); + const { code, deps } = transform(data); const fn = new Function( 'exports', @@ -84,8 +84,8 @@ export default function loader(snowpack, config) { ); const promises = deps.map(async (dep) => { - const mod = await get_module(url, dep.source); - return dep.prop ? mod[dep.prop] : dep; + const mod = await get_module(url, dep.source, url_stack); + return dep.prop ? mod[dep.prop] : mod; }); const values = await Promise.all(promises);