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

refactor(@remix-run/dev): compiler interfaces #4291

Closed
wants to merge 4 commits into from
Closed
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
64 changes: 64 additions & 0 deletions decisions/0005-remix-compiler-interface.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Remix Compiler interface

Date: 2022-08-03

Status: proposed

## Context

The ethos of Remix is "use the platform", but many popular tools and frameworks
require compiler-level support for syntax not natively supported by the browser runtime or the server runtime.

Some tools, like Typescript, are ubiquitous.
For these, we think it best to include them as part of the Remix compiler.

But there are a myriad of different CSS frameworks, a handful of different package managers, etc... that require compiler-level integration to fully support their features.

The status quo has been to assess each framework/tool and add plugins to our compiler to support each of the ones that we decide have enough upside to warrant the maintenance burden.
For example, here are PRs for [Yarn PnP support](https://github.com/remix-run/remix/pulls?q=pnp) and [CSS Modules support](https://github.com/remix-run/remix/pulls?q=%22css+modules%22).
But that places Remix releases on the critical path for support and bug fixes for each of these features!
We're already feeling some maintenance pressure from this and we don't users to have to wait for us to review and merge PRs for N different CSS frameworks before we get to the one they are using.

But Remix shouldn't care how you prefer to do CSS nor what package manager you use.
At the end of the day, Remix wants to introduce as few things on top of the platform for excellent DX and UX.
For example, Remix has strong opinions on routing that enables awesome features like loaders fetching in parallel.
But Remix does not want to prescribe your package manager.

## Considered

### Remix as a {vite,esbuild,...} plugin

Other web frameworks have instead ditched the idea of having their own compiler and instead provide plugins for popular compilers like Vite.
The idea is that users would invoke Vite (or their compiler of choice) directly.

As a proof-of-concept, we prototyped Remix plugins for Webpack.
However, Remix requires the server build to have access to the assets manifest produced by the browser build at build-time.

Coordinating the assets manifest handoff is not possible when invoking webpack directly.

### Pluggable compiler

Alternatively, we could open up our esbuild-based compiler for users to add their own esbuild plugins.

In the short term, this sounds great, but we know how this goes in the long term.
Users will add plugins and we'll have to be extremely careful whenever we change our compiler not to conflict with _any_ plugins users may be using.

## Decision

- our compiler's interface is:
- porcelain/CLI: `build`, `watch` commands
- plumbing/Node API:
- `createBrowserCompiler`, `createServerCompiler`
- from `compiler-kit`: `build`, `watch` functions

the remix build expects web standards: (#useThePlatform)
- e.g. css as stylesheets!

for features that require compiler-support (e.g. custom CSS framework),
run a pre-remix build step.
example: [vanilla extract](https://github.com/remix-run/remix/pull/4173)

## Consequences

- future work: partial build/jit (via cache)
- future work: HMR
20 changes: 20 additions & 0 deletions decisions/0006-webpack-compiler-for-migrations.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Webpack compiler for migrations

Date: 2022-08-03

Status: proposed

## Context

Most exisiting React apps are built on top of Webpack and many of those use Create React App.
We'd love for users who maintain those apps to be able to migrate to Remix incrementally.
That means supporting everything that their current setup allows from Day 1, even if they will eventually transition off of a Webpack-enable feature towards a Remix feature.

## Decision

Remix will provide a first-party, Webpack-based, pluggable compiler.
This compiler is **NOT** meant to be an alternative to the standard Remix compiler, but just a stepping stone for those migrating to Remix.
Support will be prioritized to those using this compiler to migrate an existing Webpack-based app to Remix.
Support will be limited for anyone using this compiler for greenfield apps.

## Consequences
50 changes: 50 additions & 0 deletions packages/remix-dev/compiler-kit/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
Q: who should be in charge of writing the assets manifest to disk? browser? server? build?
PC: I _think_ browser, since its written to `/public/build/`


channel -> [promise, resolve]

# compiler-kit

```ts
type Build = (remixConfig: RemixConfig, compiler: RemixCompiler) => Promise<void>

type Watch = (remixConfig: RemixConfig, createCompiler: {
options: Options,
browser: CreateCompiler<BrowserCompiler>,
server: CreateCompiler<ServerCompiler>,
}) => Promise<void>
```

notes:
- we want `compiler` as a param for `build` so that we can reuse compilers
- alternative would be for `build` to internally create a `RemixCompiler`... then we could't reuse compiler across builds.

# compiler-<implementation>

```ts
type CreateBrowserCompiler = (remixConfig: RemixConfig, options: Options) => BrowserCompiler
type CreateServerCompiler = (remixConfig: RemixConfig, options: Options) => ServerCompiler
```

# dev-server

```ts
type Serve = (
config: RemixConfig,
createCompiler: {
options: Options;
browser: CreateCompiler<BrowserCompiler>;
server: CreateCompiler<ServerCompiler>;
},
port?: number
) => Promise<void>
```

# Open questions

Q1: who should be in charge of writing the assets manifest to disk? browser? server? build?
PC: I _think_ browser, since its written to `/public/build/`


Q2: channel -> [promise, resolve]?
36 changes: 36 additions & 0 deletions packages/remix-dev/compiler-kit/build.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import fs from "fs";
import path from "path";

import type { AssetsManifest } from "../compiler/assets";
import type { RemixConfig } from "../config";
import { createChannel } from "./utils/channel";
import type { RemixCompiler } from "./interface";

// TODO error handling for if browser/server builds fail (e.g. syntax error)
// enumerate different types of errors
// console.log hints for users if we know how to diagnose the error from experience
// consider throwing custom Remix-specific error type if its an error we know more stuff about

/**
* Coordinate the hand-off of the asset manifest between the browser and server builds.
* Additionally, write the asset manifest to the file system.
*/
export const build = async (
config: RemixConfig,
compiler: RemixCompiler
): Promise<void> => {
let manifestChannel = createChannel<AssetsManifest>();
let browser = compiler.browser.build(manifestChannel);

// write manifest
manifestChannel.read().then((manifest) => {
fs.mkdirSync(config.assetsBuildDirectory, { recursive: true });
fs.writeFileSync(
path.resolve(config.assetsBuildDirectory, path.basename(manifest.url!)),
`window.__remixManifest=${JSON.stringify(manifest)};`
);
});

let server = compiler.server.build(manifestChannel);
await Promise.all([browser, server]);
};
9 changes: 9 additions & 0 deletions packages/remix-dev/compiler-kit/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export type {
BrowserCompiler,
ServerCompiler,
CreateCompiler,
RemixCompiler,
} from "./interface";
export { build } from "./build";
export { watch } from "./watch";
export type { Options } from "./options";
26 changes: 26 additions & 0 deletions packages/remix-dev/compiler-kit/interface.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import type { AssetsManifest } from "../compiler/assets";
import type { RemixConfig } from "../config";
import type { ReadChannel, WriteChannel } from "./utils/channel";
import type { Options } from "./options";

// TODO explain that `build` will be incremental (i.e. reuse compiler) if run multiple times

export interface BrowserCompiler {
// produce ./public/build/
build: (manifestChannel: WriteChannel<AssetsManifest>) => Promise<void>;
dispose: () => void;
}
export interface ServerCompiler {
// produce ./build/index.js
build: (manifestChannel: ReadChannel<AssetsManifest>) => Promise<void>;
dispose: () => void;
}
export type CreateCompiler<T extends BrowserCompiler | ServerCompiler> = (
remixConfig: RemixConfig,
options: Options
) => T;

export interface RemixCompiler {
browser: BrowserCompiler;
server: ServerCompiler;
}
15 changes: 15 additions & 0 deletions packages/remix-dev/compiler-kit/options.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
type Mode = "development" | "test" | "production";

type Target =
| "browser" // TODO: remove
| "server" // TODO: remove
| "cloudflare-workers"
| "node14";

export type Options = {
mode: Mode;
sourcemap: boolean;
target: Target;
onWarning?: (message: string, key: string) => void;
// onBuildFailure?(failure: Error | esbuild.BuildFailure): void;
};
20 changes: 20 additions & 0 deletions packages/remix-dev/compiler-kit/utils/channel.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
export interface WriteChannel<T> {
write: (data: T) => void;
}
export interface ReadChannel<T> {
read: () => Promise<T>;
}
export type Channel<T> = WriteChannel<T> & ReadChannel<T>;

export const createChannel = <T>(): Channel<T> => {
let promiseResolve: ((value: T) => void) | undefined = undefined;

let promise = new Promise<T>((resolve) => {
promiseResolve = resolve;
});

return {
write: promiseResolve!,
read: async () => promise,
};
};
137 changes: 137 additions & 0 deletions packages/remix-dev/compiler-kit/watch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
import path from "path";
import chokidar from "chokidar";

import type {
BrowserCompiler,
CreateCompiler,
RemixCompiler,
ServerCompiler,
} from "./interface";
import { build } from "./build";
import type { RemixConfig } from "../config";
import { readConfig } from "../config";
import type { Options } from "./options";

// TODO error handling for if browser/server builds fail (e.g. syntax error)
// on___Error callback
// preserve watcher for common cases, throw for exotic errors

const reloadConfig = async (config: RemixConfig): Promise<RemixConfig> => {
try {
return readConfig(config.rootDirectory);
} catch (error) {
// onBuildFailure(error as Error);
console.error(error);
throw error;
}
};

const dispose = ({ browser, server }: RemixCompiler) => {
browser.dispose();
server.dispose();
};

export const watch = async (
config: RemixConfig,
createCompiler: {
options: Options; // TODO should options be curried into createCompiler.{browser,server} ?
browser: CreateCompiler<BrowserCompiler>;
server: CreateCompiler<ServerCompiler>;
},
callbacks: {
onInitialBuild?: () => void;
onRebuildStart?: () => void;
onRebuildFinish?: (durationMs: number) => void;
onFileCreated?: (file: string) => void;
onFileChanged?: (file: string) => void;
onFileDeleted?: (file: string) => void;
} = {}
): Promise<() => Promise<void>> => {
let createRemixCompiler = (remixConfig: RemixConfig) => {
let { browser, server, options } = createCompiler;
return {
browser: browser(remixConfig, options),
server: server(remixConfig, options),
};
};
let compiler = createRemixCompiler(config);

// initial build
await build(config, compiler);
callbacks.onInitialBuild?.();

// TODO debounce
let restart = async () => {
callbacks.onRebuildStart?.();
let start = Date.now();
dispose(compiler);

config = await reloadConfig(config);
compiler = createRemixCompiler(config);
await build(config, compiler);
callbacks.onRebuildFinish?.(Date.now() - start);
};

// TODO debounce
let rebuild = async () => {
callbacks.onRebuildStart?.();
let start = Date.now();
await build(config, compiler);
callbacks.onRebuildFinish?.(Date.now() - start);
};

// watch files
let toWatch = [config.appDirectory];
if (config.serverEntryPoint) {
toWatch.push(config.serverEntryPoint);
}
config.watchPaths.forEach((watchPath) => toWatch.push(watchPath));

// what if route modules are not on filesystem?
let watcher = chokidar
.watch(toWatch, {
persistent: true,
ignoreInitial: true,
awaitWriteFinish: {
stabilityThreshold: 100,
pollInterval: 100,
},
})
.on("error", (error) => console.error(error))
.on("change", async (file) => {
callbacks.onFileChanged?.(file);
await rebuild();
})
.on("add", async (file) => {
callbacks.onFileCreated?.(file);
config = await reloadConfig(config);
if (isEntryPoint(config, file)) {
await restart();
} else {
await rebuild();
}
})
.on("unlink", async (file) => {
callbacks.onFileDeleted?.(file);
if (isEntryPoint(config, file)) {
await restart();
} else {
await rebuild();
}
});

return async () => {
await watcher.close().catch(() => undefined);
dispose(compiler);
};
};

function isEntryPoint(config: RemixConfig, file: string): boolean {
let appFile = path.relative(config.appDirectory, file);
let entryPoints = [
config.entryClientFile,
config.entryServerFile,
...Object.values(config.routes).map((route) => route.file),
];
return entryPoints.includes(appFile);
}
Loading