refactor(dev): simplify compiler #4410

pcattori · 2022-10-24T20:31:09Z

Refactored the compiler to be architected around BrowserCompiler, ServerCompiler. Notably, each of these have a compile function that can be called once for the initial build, but can be called again subsequently for rebuilds. This greatly simplifies the mental model for the Remix compiler and cuts down the code in watch. Other changes include:

Previously, we used a a ref to the assets manifest promise to coordinate the browser and server builds. The ref was also reused across rebuilds. There were checks in watch and within the serverAssetsManifestPlugin to make sure the ref was set before being accessed. Overall, this approach was error prone and hard to reason about. This has now been replaced with a Golang-style channel so that the browser build can explicitly communicate the assets manifest to the server build.
Previously, writing the assets manifest and the server build to the file system needed to be done anywhere in the code where we built the browser and server bundles. This was easy to forget and resulted in duplication. Now, the compile function for the browser and server compilers managed writing the results (including the assets manifest) to the filesystem.

Note that the build and watch APIs for the compiler stayed the same (with the minor exception of renaming onBuildFailure to onCompilerFailure).

changeset-bot · 2022-10-24T20:31:11Z

⚠️ No Changeset found

Latest commit: f644f86

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

packages/remix-dev/compiler/compile-server.ts

…ports since caching is perf improvement that is purely an implementation detail. also remove export for uncached route exports since that is not used anywhere.

using string constant unions instead of enums

…ts manifest promise

since it depends on the assets manifest

also, split out compile failure logging into its own module

…o `compiler/` directory

test was hardcoded to be skipped everytime and referenced the old rollup-based compiler and old gist app

jacob-ebey · 2022-10-28T17:48:58Z

packages/remix-dev/compiler/assets.ts

@@ -87,7 +87,7 @@ export async function createAssetsManifest(
      );
      let route = routesByFile.get(entryPointFile);
      invariant(route, `Cannot get route for entry point ${output.entryPoint}`);
-      let sourceExports = await getRouteModuleExportsCached(config, route.id);
+      let sourceExports = await getRouteModuleExports(config, route.id);


why aren't we using the cached one?

We are. I renamed getRouteModuleExportsCached to just getRouteModuleExports since we only ever use the cached version.

github-actions · 2022-10-29T07:10:04Z

🤖 Hello there,

We just published version v0.0.0-nightly-5020fa4-20221029 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

* refactor(dev): rename getRouteModuleExportsCached to getRouteModuleExports since caching is perf improvement that is purely an implementation detail. also remove export for uncached route exports since that is not used anywhere. * refactor(dev): simplify build options using string constant unions instead of enums * refactor(dev): split up browser and server builds into their own modules * refactor(dev): factor out logic for finding externals * refactor(dev): compiler interfaces for browser and server * refactor(dev): change server assets manifest plugin to accept an assets manifest promise * refactor(dev): do not compile server incrementally since it depends on the assets manifest * refactor(dev): split up build and watch into separate modules also, split out compile failure logging into its own module * refactor(dev): implement `build` function based on new remix compiler * refactor(dev): rename `BuildOptions` to `CompileOptions` and move into `compiler/` directory * refactor(dev): watch * refactor(dev): compiler index module * refactor(dev): camelCase filenames * refactor(dev): update imports and types for compiler in `commands` * test(dev): fix type errors * test(dev): remove outdated, unused test test was hardcoded to be skipped everytime and referenced the old rollup-based compiler and old gist app * refactor(dev): rename `onBuildFailure` to `onCompileFailure`

https://github.com/remix-run/remix/releases/tag/remix%401.7.0 https://github.com/remix-run/remix/releases/tag/remix%401.7.4 du -ckd1 node_modules -795648 +798524 Not much changes though I found since [1.7.5](https://github.com/remix-run/remix/releases/tag/remix%401.7.5) build started to hang when run with turbo or recursive pnpm run. Looks like in this commit some stream or connection not closed when build is finished. remix-run/remix#4410 Let's see if not fixed in newer versions and then file an issue.

remix-cla-bot bot added the CLA Signed label Oct 24, 2022

pcattori marked this pull request as ready for review October 24, 2022 21:00

pcattori requested review from mcansh and jacob-ebey October 24, 2022 21:00

pcattori marked this pull request as draft October 24, 2022 23:35

MichaelDeBoey added the package:dev label Oct 25, 2022

pcattori force-pushed the pedro/compiler-refactor branch 2 times, most recently from 6e69ead to 9867975 Compare October 25, 2022 20:15

pcattori requested a review from mjackson October 26, 2022 17:32

pcattori force-pushed the pedro/compiler-refactor branch from 808c104 to 346211f Compare October 26, 2022 17:34

pcattori commented Oct 26, 2022

View reviewed changes

packages/remix-dev/compiler/compile-server.ts Outdated Show resolved Hide resolved

pcattori force-pushed the pedro/compiler-refactor branch 5 times, most recently from bf346af to 1a67594 Compare October 27, 2022 01:43

pcattori changed the title ~~Compiler refactoring~~ refactor(dev): refactor compiler Oct 27, 2022

pcattori changed the title ~~refactor(dev): refactor compiler~~ refactor(dev): simplify compiler Oct 27, 2022

pcattori force-pushed the pedro/compiler-refactor branch 2 times, most recently from b91bdde to 225bdea Compare October 27, 2022 16:38

pcattori added 9 commits October 27, 2022 15:34

refactor(dev): rename getRouteModuleExportsCached to getRouteModuleEx…

ebaf346

…ports since caching is perf improvement that is purely an implementation detail. also remove export for uncached route exports since that is not used anywhere.

refactor(dev): simplify build options

8c79701

using string constant unions instead of enums

refactor(dev): split up browser and server builds into their own modules

b0d169e

refactor(dev): factor out logic for finding externals

b1234f7

refactor(dev): compiler interfaces for browser and server

613f2b4

refactor(dev): change server assets manifest plugin to accept an asse…

7c2aea0

…ts manifest promise

refactor(dev): do not compile server incrementally

07e4cc9

since it depends on the assets manifest

refactor(dev): split up build and watch into separate modules

2e9e0eb

also, split out compile failure logging into its own module

refactor(dev): implement build function based on new remix compiler

8d66c8e

pcattori added 8 commits October 27, 2022 15:34

refactor(dev): rename BuildOptions to CompileOptions and move int…

03efe05

…o `compiler/` directory

refactor(dev): watch

3ba075b

refactor(dev): compiler index module

d8c6965

refactor(dev): camelCase filenames

31f5528

refactor(dev): update imports and types for compiler in commands

ecb5a61

test(dev): fix type errors

5b5783c

test(dev): remove outdated, unused test

60b6b76

test was hardcoded to be skipped everytime and referenced the old rollup-based compiler and old gist app

refactor(dev): rename onBuildFailure to onCompileFailure

f644f86

pcattori force-pushed the pedro/compiler-refactor branch from 225bdea to f644f86 Compare October 27, 2022 19:34

pcattori marked this pull request as ready for review October 27, 2022 19:34

jacob-ebey approved these changes Oct 28, 2022

View reviewed changes

pcattori merged commit e44bfbf into dev Oct 28, 2022

pcattori deleted the pedro/compiler-refactor branch October 28, 2022 20:04

github-actions bot added the awaiting release This issue has been fixed and will be released soon label Oct 28, 2022

pcattori mentioned this pull request Oct 29, 2022

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

Closed

2 tasks

TrySound mentioned this pull request Dec 31, 2022

Upgrade to remix 1.7.6 webstudio-is/webstudio#725

Merged

7 tasks

TrySound mentioned this pull request Feb 22, 2023

remix build hangs after build when no tty is available. #5021

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(dev): simplify compiler #4410

refactor(dev): simplify compiler #4410

pcattori commented Oct 24, 2022 •

edited

Loading

changeset-bot bot commented Oct 24, 2022 •

edited

Loading

jacob-ebey Oct 28, 2022

pcattori Oct 28, 2022

github-actions bot commented Oct 29, 2022

refactor(dev): simplify compiler #4410

refactor(dev): simplify compiler #4410

Conversation

pcattori commented Oct 24, 2022 • edited Loading

changeset-bot bot commented Oct 24, 2022 • edited Loading

⚠️ No Changeset found

jacob-ebey Oct 28, 2022

Choose a reason for hiding this comment

pcattori Oct 28, 2022

Choose a reason for hiding this comment

github-actions bot commented Oct 29, 2022

pcattori commented Oct 24, 2022 •

edited

Loading

changeset-bot bot commented Oct 24, 2022 •

edited

Loading