Not able to deploy on Fly via Docker: `top-level await` error

binajmen commented 1 year ago

What version of Remix are you using?

v1.14.0

Are all your remix dependencies & dev-dependencies using the same version?

[X] Yes

Steps to Reproduce

Deploy to Fly using Docker

Expected Behavior

Build should succeed

Actual Behavior

Build step is failing:

#17 [build 6/6] RUN npm run build
#17 14.11 ✘ [ERROR] This require call is not allowed because the imported file "node_modules/@jspm/core/nodelibs/browser/worker_threads.js" contains a top-level await
#17 14.11 
#17 14.11     node_modules/@prisma/client/runtime/index.js:13:8230:
#17 14.11       13 │ ...ssageOnPort:LR}=require("worker_threads"),xR=["GET","HEAD","POS...
#17 14.11          ╵                            ~~~~~~~~~~~~~~~~
#17 14.11 
#17 14.11   The top-level await in "node_modules/@jspm/core/nodelibs/browser/worker_threads.js" is here:
#17 14.11 
#17 14.11     node_modules/@jspm/core/nodelibs/browser/worker_threads.js:97:49:
#17 14.11       97 │ ...rkerData, environmentData }] = await once(parentPort, 'message'));
#17 14.11          ╵                                   ~~~~~
#17 14.11 
#17 14.11 
#17 14.11 ✘ [ERROR] This require call is not allowed because the imported file "node_modules/@jspm/core/nodelibs/browser/worker_threads.js" contains a top-level await
#17 14.11 
#17 14.11     node_modules/@prisma/client/runtime/index.js:56:58637:
#17 14.11       56 │ ...MessagePort:SU}=require("worker_threads"),uA,WA=class extends E...
#17 14.11          ╵                            ~~~~~~~~~~~~~~~~
#17 14.11 
#17 14.11   The top-level await in "node_modules/@jspm/core/nodelibs/browser/worker_threads.js" is here:
#17 14.11 
#17 14.11     node_modules/@jspm/core/nodelibs/browser/worker_threads.js:97:49:
#17 14.11       97 │ ...rkerData, environmentData }] = await once(parentPort, 'message'));
#17 14.11          ╵                                   ~~~~~
#17 14.11 
#17 14.11 
#17 14.15 ERROR: "build:remix" exited with 1.

nakleiderer commented 1 year ago

This is affecting me as well when I upgrade from Remix 1.13 to 1.16 (so could be 1.14 was when it was introduced). I'm not using fly or any of the remix stacks, so this doesn't appear related to any of the templates. Our repo is loosely modeled after blues stack. The error doesn't occur when calling our own esbuild on the server file.

esbuild doesn't support top level await, but it looks like @remix/dev relies on @jspm/core via esbuild-plugin-polyfill-node. JSPM uses top-level awaits here https://github.com/jspm/jspm-core/blob/209cd168383d76215445c79001b9f45568dd92df/src-browser/worker_threads.js#L96 (and several other places)

❯ npm why @jspm/core
@jspm/core@2.0.1 dev
node_modules/@jspm/core
  @jspm/core@"^2.0.1" from esbuild-plugin-polyfill-node@0.2.0
  node_modules/esbuild-plugin-polyfill-node
    esbuild-plugin-polyfill-node@"^0.2.0" from @remix-run/dev@1.16.0
    node_modules/@remix-run/dev
      dev @remix-run/dev@"^1.16.0" from the root project

nakleiderer commented 1 year ago

I was able to circumvent the issue with an npm override:

"overrides": {
    "@remix-run/dev": {
      "esbuild-plugin-polyfill-node": "0.1.0"
    }
  }

nakleiderer commented 1 year ago

Looks like another issue popped up and it might be related to the overrides that I tried:

caught TypeError: Failed to resolve module specifier "buffer". Relative references must start with either "/", "./", or "../".

This error persists even with no routes, so it appears to be including node polyfills in the browser build. I'm not sure if this is actually related to the override or not since I can't build without that override.

bvkimball commented 1 year ago

@nakleiderer I am in the same boat, your "overrides" fixed it for me but i am also getting the "buffer" error now too.

I am pretty sure some package is getting included in the browser build as well... Will update if i get further

nakleiderer commented 1 year ago

Just saw Remix 1.16.1 dropped and gave it a shot, but receiving the same errors. I have two apps with the same error now, so maybe I can figure out a reproduction. This affects the production build and the dev builds.

nakleiderer commented 1 year ago

@bvkimball and @binajmen: Would you mind posting your package.json in a gist and linking here? I wonder if there's a common dependency causing the issue. I have 54 dependencies in common between my remix apps, hoping to narrow that down to make a reproduction based on your deps. It's a bit of a stretch, but will hopefully give more info.

bvkimball commented 1 year ago

Here are the relevant deps... i can add devDeps if you want.

https://gist.github.com/bvkimball/5f93ed2ad2f81fbd8279c71641860200

I use a mono-repo similar to this, but not the same: https://github.com/NoQuarterTeam/boilerplate

I am in the process of cleaning this up now too so i will remove any unused deps and let you know

I think its prisma or zenstack for me that is causeing it.

nakleiderer commented 1 year ago

This is what we appear to have in common:

Common dependencies:
@prisma/client
@remix-run/server-runtime
tiny-invariant
zod

Common devDependencies:
@faker-js/faker
@tailwindcss/aspect-ratio
@tailwindcss/forms
@tailwindcss/typography
@types/eslint
@types/node
@types/nprogress
@typescript-eslint/eslint-plugin
@vitest/coverage-c8
esbuild
eslint-config-prettier
eslint-plugin-tailwindcss
msw
prettier
prisma
tailwindcss
tsconfig-paths
vite-tsconfig-paths
vitest

I filtered out the dependencies used in the Remix Express template, because it works out of the box.

nakleiderer commented 1 year ago

It looks like something is causing @remix/dev to bundle the esbuild-plugin-polyfill-node. I found the generated bundle file though and inspected the source map to confirm. Still no luck on my end figuring out what is triggering it to load.

nakleiderer commented 1 year ago

Probably not helpful, but removing this line makes everything work again: https://github.com/remix-run/remix/blob/cb7a4e8fd2f3f6cf6840672905a7bf8e032849c7/packages/remix-dev/compiler/js/compiler.ts#L148

EDIT: That was only with the overrides in place. This doesn't fix it with the latest esbuild-plugin-polyfill-node version

nakleiderer commented 1 year ago

In one of my projects, I had a resource route that imported a library with node library usage. When I put that dependency behind a .server.ts file, I was able to build. This is very unexpected since it was a resource route (only exports a loader).

I also had to update a Sentry import from import { BrowserTracing } from "@sentry/tracing"; to use import * as Sentry from "@sentry/remix"; as new Sentry.BrowserTracing() but this may have been because I updated my Sentry dependencies and might not be related to the actual issues here.

I found I had more luck updating remix version-by-version instead of leaping from 1.13 > 1.16.1 directly. I was able to get better error messages for issues along the way, fix them, then upgrade to the next version.

bvkimball commented 1 year ago

Ok, thank you for all the help.... I deleted everything in my routes until it was just and index, root, and entry.x files For me it seems it is an my session.server.ts which i moved into a "package" in my monorepo then imported poorly.

I basically couldn't narrow it down to just one thing... but basically if my mono-repo packages bleed any node dep than it borked. Basically treeshaking was working or i missed something.

for me i re-exported all my monorepo packages in a file like utils/[package].server

export * from `@monorepo/[package]`;

i wish i could just mark packages as server only.

bvkimball commented 1 year ago

I really figured it what caused it for my repo.

I include zod schemas in the same package that my "server-side" business logic exists. but i also use that zod schema to do form validation on client & server-side.

I assumed that the zod schema would be "tree-shaken" out of the package, but this doesn't seem to be the case. I am using serverDependenciesToBundle feature too.

There are several other issues listing this in issues now: https://github.com/remix-run/remix/issues/6318 https://github.com/remix-run/remix/issues/6302

nakleiderer commented 1 year ago

It seems like Remix is suddenly importing files that used to be tree-shaken out of the browser build. In my other project, Sentry was also getting included despite never being used client-side. This may be specific to this kind of import: import * as Sentry from "@sentry/node";

I think Remix needs a few things to put these issues to rest:

A way to audit what is being included in the client build without fishing around in the bundles (I'm thinking a tree-like output, that shows the module import graph)
A warning when a bare module specifier ends up in the client, like "fs" or other known server runtime prefixes, like "node:*"
A fix for what appears to be a regression that occurred sometime between 1.13 and 1.16 (I think 1.14 introduced it) that led to extra things in the client bundle
Bonus points: A Remix config option to specify dependencies and files that should never be seen in the browser build. Either warn or fail the build. Maybe something like serverOnlyImports: [ "@org/package", "**/server-code/**" ]

fiws commented 1 year ago

I had the same problem and solved it by migrating some deprecated @prisma/client/runtime imports in my app to @prisma/client/runtime/library.

If you get this during startup:

imports from "@prisma/client/runtime" are deprecated.
Use "@prisma/client/runtime/library",  "@prisma/client/runtime/data-proxy" or  "@prisma/client/runtime/binary"

It might also work for you.

znycheporuk commented 1 year ago

I had a similar issue, setting target to node18 fixed an issue

remix,config.js

markdalgleish commented 1 year ago

It looks like there's a couple of solutions here:

Migrate to @prisma/client/runtime/library, as @fiws said, as this doesn't appear to call require("worker_threads").
If that doesn't work (or if you're hitting this error another way), you can move require("worker_threads") or any imports of packages/modules calling this into a *.server.ts file so that Remix won't include the code in the client module graph. Note that tree-shaking doesn't fix this problem because this error happens during an earlier phase of the build process.

It might be worth raising this as an issue upstream against esbuild-plugins-node-modules-polyfill since that plugin could potentially stub out worker_threads with an empty module in a CJS context rather than breaking the build, but this might need to be configurable via plugin options. If this is fixable upstream, we could potentially update Remix to disable worker_threads in CJS by default.

I'm closing this for now, but if this doesn't help address your problem, could you please open a new, more specific issue with a reproduction?

remix-run / remix