Skip to content

feat: Allow for custom server to participate in bundling#1504

Merged
jacob-ebey merged 31 commits into
devfrom
jacob/platform-entry
Jan 22, 2022
Merged

feat: Allow for custom server to participate in bundling#1504
jacob-ebey merged 31 commits into
devfrom
jacob/platform-entry

Conversation

@jacob-ebey

@jacob-ebey jacob-ebey commented Jan 15, 2022

Copy link
Copy Markdown
Member

feat: introduces virtual module @remix-run/assets-manifest that contains the client asset manifest
We previously wrote out a json file in the server build directory outside of the bundler. this introduced the requirement of another external bundle step for any deployment platform that requires a single file.

feat: introduces virtual module @remix-run/server-build that is the module that is currently output as the server bundle
We previously piped this into esbuild stdin, but with the introduction of customServer being able to participate in bundling we needed a way to access the "finalized" remix server build before the build finishes 🤯

feat: introduces serverBuildTarget config option and deprecates serverModuleFormat and serverPlatform
serverBuildTarget now configures defaults in the compiler for you that look like this:

serverBuildTarget serverModuleFormat serverPlatform node_modules serverBuildPath publicPath
node-cjs (default) cjs node external build/index.js /build/
arc cjs node external server/index.js /_static/build/
netlify cjs node external netlify/functions/server/index.js /build/
vercel cjs node external api/_build/index.js /build/
cloudflare-pages esm neutral bundled functions/[[path]].js /build/
cloudflare-workers esm neutral bundled build/index.js /build/

If a serverBuildTarget is set besides node-cjs and a customServer is not provided, the output module for the server build will now be a "ready to go" file for that build target. If serverBuildTarget is not defined or node-cjs the output module is the same format as today.

feat: explicitly exclude @remix-run/server-runtime, @remix-run/node, @remix-run/cloudflare-pages, @remix-run/cloudflare-workers, @remix-run/deno from client bundles. This may introduce new runtime errors in existing projects that are currently bundling server modules in the client by accident. Using xxx.server.js pattern will fix these issues for people who run into issues.

@ryanflorence

Copy link
Copy Markdown
Member

I think we need another node_modules behavior for arc where remix is bundled, but everything else is external. Unless somebody solved that problem already.

@jacob-ebey

jacob-ebey commented Jan 15, 2022

Copy link
Copy Markdown
Member Author

I think we need another node_modules behavior for arc where remix is bundled, but everything else is external. Unless somebody solved that problem already.

That's already solved. None of our outputs, bundled or not, will include an import or require to remix directly. They will all be remapped to their respective @remix-run/ packages instead in the output files.

@jacob-ebey

Copy link
Copy Markdown
Member Author

I'd like to run through the templates and make sure everything works before we merge this.... I haven't done that yet.

@ryanflorence

Copy link
Copy Markdown
Member

Good plan for a PR like this, but also we'll catch that stuff now in pre-releases! #1408

@mcansh

mcansh commented Jan 15, 2022

Copy link
Copy Markdown
Contributor

I'd like to run through the templates and make sure everything works before we merge this.... I haven't done that yet.

if you're feeling adventurous, you can gather and export all the "TEST_" prefixed ones from https://github.com/remix-run/remix/settings/secrets/actions to your shell and run node scripts/deployment-test/{target}.mjs

... or just cut an experimental and 🤞

Comment thread packages/remix-dev/config.ts Outdated

@mjackson mjackson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Initial review. Looks good! Let's go through the templates later.

Comment thread packages/remix-dev/config.ts
Comment thread packages/remix-dev/config.ts Outdated
Comment thread packages/remix-dev/compiler.ts Outdated
Comment thread packages/remix-dev/compiler.ts Outdated
Comment thread packages/remix-dev/compiler.ts Outdated
Comment thread packages/remix-dev/compiler/plugins/serverBareModulesPlugin.ts
Comment thread packages/remix-dev/compiler/plugins/serverBareModulesPlugin.ts Outdated
Comment thread packages/remix-dev/compiler/plugins/serverBareModulesPlugin.ts Outdated
Comment thread packages/remix-dev/compiler/plugins/serverRouteModulesPlugin.ts Outdated
Comment thread packages/create-remix/templates/express/remix.config.js
Comment thread packages/create-remix/templates/cloudflare-workers/package.json Outdated
Comment thread packages/create-remix/templates/vercel/package.json
- add remix config options back to convey defaults
- separated out default entries to their own files
- address PR feedback
@jacob-ebey jacob-ebey requested a review from mjackson January 20, 2022 18:42

@mjackson mjackson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Almost there! 🙌

Comment thread packages/create-remix/templates/vercel/package.json Outdated
Comment thread packages/create-remix/templates/vercel/remix.config.js
Comment thread packages/remix-dev/cli/commands.ts Outdated
Comment thread packages/remix-dev/config.ts Outdated
Comment thread packages/remix-dev/config.ts Outdated
Comment thread packages/remix-dev/compiler/plugins/serverBareModulesPlugin.ts Outdated
Comment thread packages/remix-dev/compiler/shims/cloudflare-pagesEntryModule.ts
const handleRequest = createPagesFunctionHandler({
build,
mode: process.env.NODE_ENV,
getLoadContext: context => context.env

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's add something to the docs about this. Maybe @kentcdodds can help us show people how to automatically access their durable objects in a loader or something.

Comment thread packages/remix-server-build/index.ts Outdated
Comment thread packages/remix-dev/tsconfig.json Outdated
- removed vercel specific logic from the compiler
- update customServer config name in internal config
@jacob-ebey jacob-ebey requested a review from mjackson January 20, 2022 22:10
@kentcdodds

Copy link
Copy Markdown
Member

Let's talk about this tomorrow, but I think if we can move all the vendor-specific code to the adapters that would be important. @jacob-ebey mentioned @mjackson and he had already discussed this, but I would like to push back on the decision to include hosting provider code in our compiler. If any of these hosting providers change where they want the output or anything else we need to do a major version bump of the compiler (think Babel pre-v6 when they included all the transform plugins within babel itself). Not a great situation to be in when we don't control the breaking changes of our own compiler code 😬

@mjackson

Copy link
Copy Markdown
Member

That's a great point @kentcdodds. I spoke with @jacob-ebey about it and I think we'll just remove the defaults server entry code from our compiler. Instead, we'll just dump it into the project directly when someone runs create-remix (same as today) except now that file will be part of the build so they won't need a separate build step for it.

@kentcdodds

Copy link
Copy Markdown
Member

Sounds much better. Just don't want a host to trigger a breaking change in our compiler 👍

Comment thread packages/create-remix/templates/netlify/server.js Outdated
Comment thread packages/create-remix/templates/vercel/remix.config.js Outdated
Comment thread packages/remix-dev/config.ts Outdated
Comment thread packages/remix-dev/config.ts Outdated
Comment thread packages/remix-dev/compiler/virtualModules.ts Outdated
- rename customServer to just server
@jacob-ebey jacob-ebey merged commit 505d087 into dev Jan 22, 2022
@jacob-ebey jacob-ebey deleted the jacob/platform-entry branch January 22, 2022 03:03
@mjackson

Copy link
Copy Markdown
Member

Nice work getting this done, @jacob-ebey! Can't wait to release this next week 🙌

@lpsinger

Copy link
Copy Markdown
Contributor

Nifty. Does this also address https://discord.com/channels/770287896669978684/935307164308881419?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants