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

Support relative imports #40

Merged
merged 27 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
d13dbc8
support relative imports
cinxmo Oct 20, 2023
640db4f
progress?
cinxmo Oct 23, 2023
db1a051
more progress part 2?
cinxmo Oct 26, 2023
6e976e7
Merge branch 'main' into fix-import-rewriting
cinxmo Oct 29, 2023
e8e5e3c
sourcePath within root, require sourcePath, add FileAttachment into f…
cinxmo Oct 30, 2023
0d6033c
delete unused constants.js
cinxmo Oct 30, 2023
1222cba
update tests
cinxmo Oct 30, 2023
68456bb
address feedback and add tests
cinxmo Oct 31, 2023
909f798
Merge branch 'main' into fix-import-rewriting
cinxmo Oct 31, 2023
294b484
address comments
cinxmo Nov 1, 2023
a58687c
remove leading slash if it exists
cinxmo Nov 1, 2023
ab5439d
address comments from pairing session
cinxmo Nov 2, 2023
e276389
Merge branch 'main' into fix-import-rewriting
cinxmo Nov 2, 2023
1a33113
small fix
cinxmo Nov 2, 2023
ae9486d
changes
cinxmo Nov 3, 2023
ea807d0
Merge branch 'main' into fix-import-rewriting
cinxmo Nov 3, 2023
0fc3aa3
do not throw error if file is not a data loader
cinxmo Nov 3, 2023
e733072
revert unnecessary changes
cinxmo Nov 4, 2023
f996813
Merge branch 'main' into fix-import-rewriting
cinxmo Nov 6, 2023
4d16092
address feedback, move static-import-npm to imports test directory
cinxmo Nov 7, 2023
b1cdf2f
add test cases for isLocalImport
cinxmo Nov 7, 2023
fd9b860
add ending slash
cinxmo Nov 7, 2023
be546da
Merge branch 'main' into fix-import-rewriting
cinxmo Nov 7, 2023
00e064c
add comment and modify getImportPreloads
cinxmo Nov 7, 2023
55d068d
add local imports to preloads list
cinxmo Nov 7, 2023
22ce01e
Update test/input/imports/bar.js
cinxmo Nov 7, 2023
2ced0f6
Merge branch 'main' into fix-import-rewriting
cinxmo Nov 7, 2023
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
2 changes: 1 addition & 1 deletion src/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ async function build(context: CommandContext) {
pages,
resolver
});
files.push(...render.files.map((f) => join(sourceFile, "..", f.name)));
files.push(...render.files.map((f) => f.name));
await prepareOutput(outputPath);
await writeFile(outputPath, render.html);
}
Expand Down
28 changes: 20 additions & 8 deletions src/javascript.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Parser, tokTypes, type Options} from "acorn";
import {Parser, tokTypes, type Node, type Options} from "acorn";
import mime from "mime";
import {findAwaits} from "./javascript/awaits.js";
import {findDeclarations} from "./javascript/declarations.js";
Expand All @@ -20,6 +20,7 @@ export interface FileReference {

export interface ImportReference {
name: string;
type: "global" | "local";
}

export interface Transpile {
Expand All @@ -36,13 +37,14 @@ export interface Transpile {
export interface ParseOptions {
id: string;
root: string;
sourcePath: string;
inline?: boolean;
sourceLine?: number;
globals?: Set<string>;
}

export function transpileJavaScript(input: string, options: ParseOptions): Transpile {
const {id} = options;
const {id, root, sourcePath} = options;
try {
const node = parseJavaScript(input, options);
const databases = node.features.filter((f) => f.type === "DatabaseClient").map((f) => ({name: f.name}));
Expand All @@ -57,8 +59,8 @@ export function transpileJavaScript(input: string, options: ParseOptions): Trans
output.insertRight(input.length, "\n))");
inputs.push("display");
}
rewriteImports(output, node);
rewriteFetches(output, node);
rewriteImports(output, node, root, sourcePath);
rewriteFetches(output, node, root, sourcePath);
return {
id,
...(inputs.length ? {inputs} : null),
Expand Down Expand Up @@ -99,8 +101,18 @@ function trim(output: Sourcemap, input: string): void {

export const parseOptions: Options = {ecmaVersion: 13, sourceType: "module"};

export function parseJavaScript(input: string, options: ParseOptions) {
const {globals = defaultGlobals, inline = false, root} = options;
export interface JavaScriptNode {
body: Node;
declarations: {name: string}[] | null;
references: {name: string}[];
features: {type: unknown; name: string}[];
imports: {type: "global" | "local"; name: string}[];
expression: boolean;
async: boolean;
}

function parseJavaScript(input: string, options: ParseOptions): JavaScriptNode {
const {globals = defaultGlobals, inline = false, root, sourcePath} = options;
// First attempt to parse as an expression; if this fails, parse as a program.
let expression = maybeParseExpression(input, parseOptions);
if (expression?.type === "ClassExpression" && expression.id) expression = null; // treat named class as program
Expand All @@ -109,8 +121,8 @@ export function parseJavaScript(input: string, options: ParseOptions) {
const body = expression ?? (Parser.parse(input, parseOptions) as any);
const references = findReferences(body, globals, input);
const declarations = expression ? null : findDeclarations(body, globals, input);
const features = findFeatures(body, references, input);
const imports = findImports(body, root);
const {imports, features: importFeatures} = findImports(body, root, sourcePath);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't really like how it returns features and imports in a function called "findImports" 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

We will also have additional features like Secrets soon, so we should anticipate that if we can.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed a ticket to address the confusion around adding local fetches and local imports as file attachments: #87

const features = [...importFeatures, ...findFeatures(body, root, sourcePath, references, input)];
return {
body,
declarations,
Expand Down
28 changes: 8 additions & 20 deletions src/javascript/features.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import {simple} from "acorn-walk";
import {syntaxError} from "./syntaxError.js";
import {isLocalImport} from "./imports.ts";
import {dirname, join} from "node:path";

export function findFeatures(node, references, input) {
export function findFeatures(node, root, sourcePath, references, input) {
const features = [];

simple(node, {
Expand All @@ -11,10 +13,9 @@ export function findFeatures(node, references, input) {
arguments: args,
arguments: [arg]
} = node;

// Promote fetches with static literals to file attachment references.
if (isLocalFetch(node, references)) {
features.push({type: "FileAttachment", name: getStringLiteralValue(arg)});
if (isLocalFetch(node, references, root, sourcePath)) {
features.push({type: "FileAttachment", name: join(dirname(sourcePath), getStringLiteralValue(arg))});
cinxmo marked this conversation as resolved.
Show resolved Hide resolved
return;
}

Expand All @@ -33,27 +34,14 @@ export function findFeatures(node, references, input) {
if (args.length !== 1 || !isStringLiteral(arg)) {
throw syntaxError(`${callee.name} requires a single literal string argument`, node, input);
}

features.push({type: callee.name, name: getStringLiteralValue(arg)});
},
// Promote dynamic imports with static literals to file attachment references.
ImportExpression: findImport,
ImportDeclaration: findImport
});

function findImport(node) {
if (isStringLiteral(node.source)) {
const value = getStringLiteralValue(node.source);
if (value.startsWith("./")) {
features.push({type: "FileAttachment", name: value});
}
}
}
});

return features;
}

export function isLocalFetch(node, references) {
export function isLocalFetch(node, references, root, sourcePath) {
if (node.type !== "CallExpression") return false;
const {
callee,
Expand All @@ -65,7 +53,7 @@ export function isLocalFetch(node, references) {
!references.includes(callee) &&
arg &&
isStringLiteral(arg) &&
getStringLiteralValue(arg).startsWith("./")
isLocalImport(getStringLiteralValue(arg), root, sourcePath)
);
}

Expand Down
6 changes: 3 additions & 3 deletions src/javascript/fetches.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import {simple} from "acorn-walk";
import {isLocalFetch} from "./features.js";

export function rewriteFetches(output, root) {
simple(root.body, {
export function rewriteFetches(output, rootNode, root, sourcePath) {
simple(rootNode.body, {
CallExpression(node) {
if (isLocalFetch(node, root.references)) {
if (isLocalFetch(node, rootNode.references, root, sourcePath)) {
output.insertLeft(node.arguments[0].start + 3, "_file/");
}
}
Expand Down
55 changes: 37 additions & 18 deletions src/javascript/imports.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import {Parser} from "acorn";
import type {Node} from "acorn";
import {simple} from "acorn-walk";
import {readFileSync} from "node:fs";
import {dirname, join, relative, resolve} from "node:path";
import {parseOptions} from "../javascript.js";
import {dirname, join} from "node:path";
import {type JavaScriptNode, parseOptions} from "../javascript.js";
import {getStringLiteralValue, isStringLiteral} from "./features.js";

export function findImports(body, root) {
const imports: {name: string}[] = [];
export function findImports(body: Node, root: string, sourcePath: string) {
const imports: {name: string; type: "global" | "local"}[] = [];
const features: {name: string; type: string}[] = [];
const paths = new Set<string>();

simple(body, {
Expand All @@ -19,8 +21,11 @@ export function findImports(body, root) {
function findImport(node) {
if (isStringLiteral(node.source)) {
const value = getStringLiteralValue(node.source);
if (value.startsWith("./")) findLocalImports(join(root, value));
imports.push({name: value});
if (isLocalImport(value, root, sourcePath)) {
findLocalImports(join(dirname(sourcePath), value));
} else {
imports.push({name: value, type: "global"});
}
}
}

Expand All @@ -29,8 +34,10 @@ export function findImports(body, root) {
function findLocalImports(path) {
if (paths.has(path)) return;
paths.add(path);
imports.push({type: "local", name: path});
features.push({type: "FileAttachment", name: path});
try {
const input = readFileSync(path, "utf-8");
const input = readFileSync(join(root, path), "utf-8");
const program = Parser.parse(input, parseOptions);
simple(program, {
ImportDeclaration: findLocalImport,
Expand All @@ -44,38 +51,41 @@ export function findImports(body, root) {
function findLocalImport(node) {
if (isStringLiteral(node.source)) {
const value = getStringLiteralValue(node.source);
if (value.startsWith("./")) {
const subpath = resolve(dirname(path), value);
if (isLocalImport(value, root, path)) {
const subpath = join(dirname(path), value);
findLocalImports(subpath);
imports.push({name: `./${relative(root, subpath)}`});
} else {
imports.push({name: value});
imports.push({name: value, type: "global"});
// non-local imports don't need to be promoted to file attachments
Copy link
Contributor

Choose a reason for hiding this comment

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

Are the functions findImport() and findLocalImport() are the same (?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

findImport will also include names of third party modules (npm packages) that we import via their CDN

}
}
}
}

return imports;
return {imports, features};
}

// TODO parallelize multiple static imports
// TODO need to know the local path of the importing notebook; this assumes it’s in the root
export function rewriteImports(output, root) {
simple(root.body, {
export function rewriteImports(output: any, rootNode: JavaScriptNode, root: string, sourcePath: string) {
simple(rootNode.body, {
ImportExpression(node: any) {
if (isStringLiteral(node.source)) {
const value = getStringLiteralValue(node.source);
output.replaceLeft(
node.source.start,
node.source.end,
JSON.stringify(value.startsWith("./") ? `/_file/${value.slice(2)}` : resolveImport(value))
JSON.stringify(
isLocalImport(value, root, sourcePath)
? join("/_file/", join(dirname(sourcePath), value))
: resolveImport(value)
)
);
}
},
ImportDeclaration(node: any) {
if (isStringLiteral(node.source)) {
const value = getStringLiteralValue(node.source);
root.async = true;
rootNode.async = true;
output.replaceLeft(
node.start,
node.end,
Expand All @@ -86,7 +96,9 @@ export function rewriteImports(output, root) {
? node.specifiers.find(isNamespaceSpecifier).local.name
: "{}"
} = await import(${JSON.stringify(
value.startsWith("./") ? `/_file/${value.slice(2)}` : resolveImport(value)
isLocalImport(value, root, sourcePath)
? join("/_file/", join(dirname(sourcePath), value))
: resolveImport(value)
)});`
);
}
Expand All @@ -102,6 +114,13 @@ function rewriteImportSpecifier(node) {
: `${node.imported.name}: ${node.local.name}`;
}

export function isLocalImport(value: string, root: string, sourcePath: string): boolean {
return (
["./", "../", "/"].some((prefix) => value.startsWith(prefix)) &&
join(root + "/", dirname(sourcePath), value).startsWith(root)
);
}

function isNamespaceSpecifier(node) {
return node.type === "ImportNamespaceSpecifier";
}
Expand Down
16 changes: 9 additions & 7 deletions src/markdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ function uniqueCodeId(context: ParseContext, content: string): string {
return id;
}

function makeFenceRenderer(root: string, baseRenderer: RenderRule): RenderRule {
function makeFenceRenderer(root: string, baseRenderer: RenderRule, sourcePath: string): RenderRule {
return (tokens, idx, options, context: ParseContext, self) => {
const token = tokens[idx];
const [language, option] = token.info.split(" ");
Expand All @@ -88,6 +88,7 @@ function makeFenceRenderer(root: string, baseRenderer: RenderRule): RenderRule {
const transpile = transpileJavaScript(token.content, {
id,
root,
sourcePath,
sourceLine: context.startLine + context.currentLine
});
extendPiece(context, {code: [transpile]});
Expand Down Expand Up @@ -235,13 +236,14 @@ const transformPlaceholderCore: RuleCore = (state) => {
state.tokens = output;
};

function makePlaceholderRenderer(root: string): RenderRule {
function makePlaceholderRenderer(root: string, sourcePath: string): RenderRule {
return (tokens, idx, options, context: ParseContext) => {
const id = uniqueCodeId(context, tokens[idx].content);
const token = tokens[idx];
const transpile = transpileJavaScript(token.content, {
id,
root,
sourcePath,
inline: true,
sourceLine: context.startLine + context.currentLine
});
Expand Down Expand Up @@ -331,7 +333,7 @@ function toParseCells(pieces: RenderPiece[]): CellPiece[] {
return cellPieces;
}

export function parseMarkdown(source: string, root: string): ParseResult {
export function parseMarkdown(source: string, root: string, sourcePath: string): ParseResult {
const parts = matter(source);
// TODO: We need to know what line in the source the markdown starts on and pass that
// as startLine in the parse context below.
Expand All @@ -351,8 +353,8 @@ export function parseMarkdown(source: string, root: string): ParseResult {
md.use(MarkdownItAnchor, {permalink: MarkdownItAnchor.permalink.headerLink({class: "observablehq-header-anchor"})});
md.inline.ruler.push("placeholder", transformPlaceholderInline);
md.core.ruler.before("linkify", "placeholder", transformPlaceholderCore);
md.renderer.rules.placeholder = makePlaceholderRenderer(root);
md.renderer.rules.fence = makeFenceRenderer(root, md.renderer.rules.fence!);
md.renderer.rules.placeholder = makePlaceholderRenderer(root, sourcePath);
md.renderer.rules.fence = makeFenceRenderer(root, md.renderer.rules.fence!, sourcePath);
md.renderer.rules.softbreak = makeSoftbreakRenderer(md.renderer.rules.softbreak!);
md.renderer.render = renderIntoPieces(md.renderer, root);
const context: ParseContext = {files: [], imports: [], pieces: [], startLine: 0, currentLine: 0};
Expand Down Expand Up @@ -441,6 +443,6 @@ export function diffMarkdown({parse: prevParse}: ReadMarkdownResult, {parse: nex
}

export async function readMarkdown(path: string, root: string): Promise<ReadMarkdownResult> {
const contents = await readFile(path, "utf-8");
return {contents, parse: parseMarkdown(contents, root), hash: computeHash(contents)};
const contents = await readFile(pathFromRoot(path, root)!, "utf-8");
return {contents, parse: parseMarkdown(contents, root, path), hash: computeHash(contents)};
}
2 changes: 1 addition & 1 deletion src/navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export async function readPages(root: string): Promise<NonNullable<RenderOptions
if (extname(file) !== ".md") continue;
let parsed: ParseResult;
try {
parsed = parseMarkdown(await readFile(join(root, file), "utf-8"), root);
parsed = parseMarkdown(await readFile(join(root, file), "utf-8"), root, file);
} catch (error) {
if (!isNodeError(error) || error.code !== "ENOENT") throw error; // internal error
continue;
Expand Down
4 changes: 2 additions & 2 deletions src/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,8 @@ function handleWatch(socket: WebSocket, options: {root: string; resolver: CellRe
let {path} = message;
if (normalize(path).startsWith("..")) throw new Error("Invalid path: " + path);
if (path.endsWith("/")) path += "index";
path = join(root, normalize(path) + ".md");
markdownWatcher = watch(path, await refreshMarkdown(path));
path = normalize(path) + ".md";
markdownWatcher = watch(join(root, path), await refreshMarkdown(path));
break;
}
}
Expand Down
Loading