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

feat(compiler): officially support Deno #3117

Merged
merged 8 commits into from
May 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
**/tests/__snapshots/
**/node_modules/
!.eslintrc.js
templates/deno
.tmp
/playground
**/__tests__/fixtures

# deno
packages/remix-deno
templates/deno
13 changes: 13 additions & 0 deletions .vscode/deno_resolve_npm_imports.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"// Resolve NPM imports for `packages/remix-deno`.": "",

"// This import map is used solely for the denoland.vscode-deno extension.": "",
"// Remix does not support import maps.": "",
"// Dependency management is done through `npm` and `node_modules/` instead.": "",
"// Deno-only dependencies may be imported via URL imports (without using import maps).": "",

"imports": {
"mime": "https://esm.sh/mime@3.0.0",
"@remix-run/server-runtime": "https://esm.sh/@remix-run/server-runtime@1.4.3"
}
}
6 changes: 5 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
{
"typescript.tsdk": "node_modules/typescript/lib"
"typescript.tsdk": "node_modules/typescript/lib",
"deno.enablePaths": [
"./packages/remix-deno/",
],
"deno.importMap": "./.vscode/deno_resolve_npm_imports.json"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
# Use `npm` to manage NPM dependencies for Deno projects

Date: 2022-05-10

Status: accepted

## Context

Deno has three ways to manage dependencies:

1. Inlined URL imports: `import {...} from "https://deno.land/x/blah"`
2. [deps.ts](https://deno.land/manual/examples/manage_dependencies)
3. [Import maps](https://deno.land/manual/linking_to_external_code/import_maps)

Additionally, NPM packages can be accessed as Deno modules via [Deno-friendly CDNs](https://deno.land/manual/node/cdns#deno-friendly-cdns) like https://esm.sh .

Remix has some requirements around dependencies:
- Remix treeshakes dependencies that are free of side-effects.
- Remix sets the environment (dev/prod/test) across all code, including dependencies, at runtime via the `NODE_ENV` environment variable.
- Remix depends on some NPM packages that should be specified as peer dependencies (notably, `react` and `react-dom`).

### Treeshaking

To optimize bundle size, Remix [treeshakes](https://esbuild.github.io/api/#tree-shaking) your app's code and dependencies.
This also helps to separate browser code and server code.

Under the hood, the Remix compiler uses [esbuild](https://esbuild.github.io).
Like other bundlers, `esbuild` uses [`sideEffects` in `package.json` to determine when it is safe to eliminate unused imports](https://esbuild.github.io/api/#conditionally-injecting-a-file).

Unfortunately, URL imports do not have a standard mechanism for marking packages as side-effect free.

### Setting dev/prod/test environment

Deno-friendly CDNs set the environment via a query parameter (e.g. `?dev`), not via an environment variable.
That means changing environment requires changing the URL import in the source code.
While you could use multiple import maps (`dev.json`, `prod.json`, etc...) to workaround this, import maps have other limitations:

- standard tooling for managing import maps is not available
- import maps are not composeable, so any dependencies that use import maps must be manually accounted for

### Specifying peer dependencies

Even if import maps were perfected, CDNs compile each dependency in isolation.
That means that specifying peer dependencies becomes tedious and error-prone as the user needs to:

- determine which dependencies themselves depend on `react` (or other similar peer dependency), even if indirectly.
- manually figure out which `react` version works across _all_ of these dependencies
- set that version for `react` as a query parameter in _all_ or the URLs for the identified dependencies

If any dependencies change (added, removed, version change),
the user must repeat all of these steps again.

## Decision

### Use `npm` to manage NPM dependencies for Deno

Do not use Deno-friendly CDNs for NPM dependencies in Remix projects using Deno.

Use `npm` and `node_modules/` to manage NPM dependencies like `react` for Remix projects, even when using Deno with Remix.

Deno module dependencies (e.g. from `https://deno.land`) can still be managed via URL imports.

### Allow URL imports

Remix will preserve any URL imports in the built bundles as external dependencies,
letting your browser runtime and server runtime handle them accordingly.
That means that you may:

- use URL imports for the browser
- use URL imports for the server, if your server runtime supports it

For example, Node will throw errors for URL imports, while Deno will resolve URL imports as normal.

### Do not support import maps

Remix will not yet support import maps.

## Consequences

- URL imports will not be treeshaken
- Users can specify environment via the `NODE_ENV` environment variable at runtime.
- Users won't have to do error-prone, manual dependency resolution.

### VS Code type hints

Users may configure an import map for the [Deno extension for VS Code](denoland.vscode-deno) to enable type hints for NPM-managed dependencies within their Deno editor:

`.vscode/resolve_npm_imports_in_deno.json`
```json
{
"// This import map is used solely for the denoland.vscode-deno extension.": "",
"// Remix does not support import maps.": "",
"// Dependency management is done through `npm` and `node_modules/` instead.": "",
"// Deno-only dependencies may be imported via URL imports (without using import maps).": "",

"imports": {
"react": "https://esm.sh/react@18.0.0",
"react-dom": "https://esm.sh/react-dom@18.0.0",
"react-dom/server": "https://esm.sh/react-dom@18.0.0/server"
}
}
```

`.vscode/settings.json`
```json
{
"deno.enable": true,
"deno.importMap": "./.vscode/resolve_npm_imports_in_deno.json"
}
```
230 changes: 230 additions & 0 deletions integration/deno-compiler-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,230 @@
import { test, expect } from "@playwright/test";
import * as fse from "fs-extra";
import path from "path";
import shell from "shelljs";
import glob from "glob";

import { createFixtureProject, js, json } from "./helpers/create-fixture";

let projectDir: string;

const findBrowserBundle = (projectDir: string): string =>
path.resolve(projectDir, "public", "build");

const findServerBundle = (projectDir: string): string =>
path.resolve(projectDir, "build", "index.js");

const importPattern = (importSpecifier: string) =>
new RegExp(
String.raw`import\s*{.*}\s*from\s*"` + importSpecifier + String.raw`"`
);

const findCodeFiles = async (directory: string) =>
glob.sync("**/*.@(js|jsx|ts|tsx)", {
cwd: directory,
absolute: true,
});
const searchFiles = async (pattern: string | RegExp, files: string[]) => {
let result = shell.grep("-l", pattern, files);
return result.stdout
.trim()
.split("\n")
.filter((line) => line.length > 0);
};

test.beforeAll(async () => {
projectDir = await createFixtureProject({
template: "deno-template",
files: {
"package.json": json({
private: true,
sideEffects: false,
dependencies: {
"@remix-run/deno": "0.0.0-local-version",
"@remix-run/react": "0.0.0-local-version",
react: "0.0.0-local-version",
"react-dom": "0.0.0-local-version",
component: "0.0.0-local-version",
"deno-pkg": "0.0.0-local-version",
},
devDependencies: {
"@remix-run/dev": "0.0.0-local-version",
},
}),
"app/routes/index.jsx": js`
import fake from "deno-pkg";
import { urlComponent } from "https://deno.land/x/component.ts";
import { urlUtil } from "https://deno.land/x/util.ts";
import { urlServerOnly } from "https://deno.land/x/server-only.ts";
import { npmComponent } from "npm-component";
import { npmUtil } from "npm-util";
import { npmServerOnly } from "npm-server-only";
import { useLoaderData } from "@remix-run/react";
export const loader = () => {
return json({
a: urlUtil(),
b: urlServerOnly(),
c: npmUtil(),
d: npmServerOnly(),
});
}
export default function Index() {
const data = useLoaderData();
return (
<ul>
<li>{fake}</li>
<li>{urlComponent}</li>
<li>{urlUtil()}</li>
<li>{data.a}</li>
<li>{data.b}</li>
<li>{npmComponent}</li>
<li>{npmUtil()}</li>
<li>{data.c}</li>
<li>{data.d}</li>
</ul>
)
}
`,
"node_modules/npm-component/package.json": json({
name: "npm-component",
version: "1.0.0",
sideEffects: false,
}),
"node_modules/npm-component/index.js": js`
module.exports = { npmComponent: () => "NPM_COMPONENT" };
`,
"node_modules/npm-util/package.json": json({
name: "npm-util",
version: "1.0.0",
sideEffects: false,
}),
"node_modules/npm-util/index.js": js`
module.exports = { npmUtil: () => "NPM_UTIL" };
`,
"node_modules/npm-server-only/package.json": json({
name: "npm-server-only",
version: "1.0.0",
sideEffects: false,
MichaelDeBoey marked this conversation as resolved.
Show resolved Hide resolved
}),
"node_modules/npm-server-only/index.js": js`
module.exports = { npmServerOnly: () => "NPM_SERVER_ONLY" };
`,
"node_modules/deno-pkg/package.json": json({
name: "deno-pkg",
version: "1.0.0",
type: "module",
main: "./default.js",
exports: {
deno: "./deno.js",
worker: "./worker.js",
default: "./default.js",
},
sideEffects: false,
}),
"node_modules/deno-pkg/deno.js": js`
export default "DENO_EXPORTS";
`,
"node_modules/deno-pkg/worker.js": js`
export default "WORKER_EXPORTS";
`,
"node_modules/deno-pkg/default.js": js`
export default "DEFAULT_EXPORTS";
`,
},
});
});

test("compiler does not bundle url imports for server", async () => {
let serverBundle = await fse.readFile(findServerBundle(projectDir), "utf8");
expect(serverBundle).toMatch(importPattern("https://deno.land/x/util.ts"));
expect(serverBundle).toMatch(
importPattern("https://deno.land/x/server-only.ts")
);

// server-side rendering
expect(serverBundle).toMatch(
importPattern("https://deno.land/x/component.ts")
);
});

test("compiler does not bundle url imports for browser", async () => {
let browserBundle = findBrowserBundle(projectDir);
let browserCodeFiles = await findCodeFiles(browserBundle);

let utilFiles = await searchFiles(
importPattern("https://deno.land/x/util.ts"),
browserCodeFiles
);
expect(utilFiles.length).toBeGreaterThanOrEqual(1);

let componentFiles = await searchFiles(
importPattern("https://deno.land/x/component.ts"),
browserCodeFiles
);
expect(componentFiles.length).toBeGreaterThanOrEqual(1);

/*
Url imports _could_ have side effects, but the vast majority do not.
Currently Remix marks all URL imports as side-effect free.
*/
let serverOnlyUtilFiles = await searchFiles(
importPattern("https://deno.land/x/server-only.ts"),
browserCodeFiles
);
expect(serverOnlyUtilFiles.length).toBe(0);
});

test("compiler bundles npm imports for server", async () => {
let serverBundle = await fse.readFile(findServerBundle(projectDir), "utf8");

expect(serverBundle).not.toMatch(importPattern("npm-component"));
expect(serverBundle).toContain("NPM_COMPONENT");

expect(serverBundle).not.toMatch(importPattern("npm-util"));
expect(serverBundle).toContain("NPM_UTIL");

expect(serverBundle).not.toMatch(importPattern("npm-server-only"));
expect(serverBundle).toContain("NPM_SERVER_ONLY");
});

test("compiler bundles npm imports for browser", async () => {
let browserBundle = findBrowserBundle(projectDir);
let browserCodeFiles = await findCodeFiles(browserBundle);

let utilImports = await searchFiles(
importPattern("npm-util"),
browserCodeFiles
);
expect(utilImports.length).toBe(0);
let utilFiles = await searchFiles("NPM_UTIL", browserCodeFiles);
expect(utilFiles.length).toBeGreaterThanOrEqual(1);

let componentImports = await searchFiles(
importPattern("npm-component"),
browserCodeFiles
);
expect(componentImports.length).toBe(0);
let componentFiles = await searchFiles("NPM_COMPONENT", browserCodeFiles);
expect(componentFiles.length).toBeGreaterThanOrEqual(1);

let serverOnlyImports = await searchFiles(
importPattern("npm-server-only"),
browserCodeFiles
);
expect(serverOnlyImports.length).toBe(0);
let serverOnlyFiles = await searchFiles("NPM_SERVER_ONLY", browserCodeFiles);
expect(serverOnlyFiles.length).toBe(0);
});

test("compiler bundles deno export of 3rd party package", async () => {
let serverBundle = await fse.readFile(findServerBundle(projectDir), "utf8");

expect(serverBundle).toMatch("DENO_EXPORTS");
expect(serverBundle).not.toMatch("DEFAULT_EXPORTS");
});
2 changes: 1 addition & 1 deletion integration/helpers/create-fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ interface FixtureInit {
buildStdio?: Writable;
sourcemap?: boolean;
files: { [filename: string]: string };
template?: "cf-template" | "node-template";
template?: "cf-template" | "deno-template" | "node-template";
setup?: "node" | "cloudflare";
}

Expand Down
5 changes: 5 additions & 0 deletions integration/helpers/deno-template/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/node_modules/

/.cache
/build
/public/build
Loading