Adding serverDependenciesToBundle causes build not to find other modules

garth commented 2 years ago

What version of Remix are you using?

1.4.1

Steps to Reproduce

When a warning is shown that a package needs to be added to serverDependenciesToBundle, I add the package the config. But then the build fails on every other dependency with "Cannot find module".

Error: Cannot find module 'async-lock'

Removing the serverDependenciesToBundle from the config fixes the build but shows ESM warnings.

I am installing dependencies with pnpm.

Expected Behavior

Adding serverDependenciesToBundle to config should remove ESM warnings and not break the build.

Actual Behavior

Adding serverDependenciesToBundle to config does fix the ESM warnings, but causes build to fail.

garth commented 2 years ago

Reverting back to v1.3.4 and there are no ESM warnings and the build works without serverDependenciesToBundle.

sdalonzo commented 2 years ago

I encountered a similar problem, and the downgrade to v1.3.4 as suggested here was the only fix that worked.

While using v1.4.1, here's what I observed:

Our monorepo fails builds with warnings by design, so I set up serverDependenciesToBundle to get my builds passing
- For our project, this is @apollo dependencies
The project built successfully locally, in CI, and in the job that produces the deployable artifact
Once the artifact is deployed and the app is started, then it fails in the Cannot find module pattern:
- remix app depends on internal npm project A, installs date-fns as a dependency to satisfy A's peer
- A has date-fns as a dev + peer dependency
- Remix app Cannot find module: date-fns at runtime and fails to start

Reverting to v1.3.4 (without magic remix package) works fine.

mcansh commented 2 years ago

👋 hey @garth and @sdalonzo can one of you create a simple reproducible stackblitz, codesandbox, or pr using the bug report for me 🙏

craigpalermo commented 2 years ago

Hey @mcansh, I created a repo with a new Remix project to reproduce the issue that @sdalonzo reported. This commit shows how installing @apollo/client and using one of its exports produces the warning that recommends adding it to serverDependenciesToBundle, which then requires adding more entries to serverDependenciesToBundle.

kiliman commented 2 years ago

I have a script that will find all the ESM packages for you. https://github.com/kiliman/rmx-cli#-get-esm-packages

❯ rmx get-esm-packages @apollo/client
📦 @apollo/client
📦 ts-invariant
📦 zen-observable-ts

🔨 Add the following dependencies to your serverDependenciesToBundle

"@apollo/client",
"ts-invariant",
"zen-observable-ts"

mcansh commented 2 years ago

awesome @kiliman! our current serverDependenciesToBundle only looks at bundling the explicit (or regex) dependency, so you'll have to also add any transitive dependencies as well

songkeys commented 2 years ago

I can't believe I need to include all the transitive dependencies! That's a huge list. Could we make this more smooth?

(Below only shows the result of one of my deps.)

❯ npx rmx-cli get-esm-packages "@milkdown/core"                                                                 
📦 @milkdown/core
📦 @milkdown/ctx
📦 @milkdown/exception
📦 @milkdown/transformer
📦 remark
📦 remark-parse
📦 mdast-util-from-markdown
📦 decode-named-character-reference
📦 character-entities
📦 mdast-util-to-string
📦 micromark
📦 micromark-core-commonmark
📦 micromark-factory-destination
📦 micromark-util-character
📦 micromark-util-symbol
📦 micromark-util-types
📦 micromark-factory-label
📦 micromark-factory-space
📦 micromark-factory-title
📦 micromark-factory-whitespace
📦 micromark-util-chunked
📦 micromark-util-classify-character
📦 micromark-util-html-tag-name
📦 micromark-util-normalize-identifier
📦 micromark-util-resolve-all
📦 micromark-util-subtokenize
📦 micromark-util-combine-extensions
📦 micromark-util-decode-numeric-character-reference
📦 micromark-util-encode
📦 micromark-util-sanitize-uri
📦 micromark-util-decode-string
📦 unist-util-stringify-position
📦 unified
📦 bail
📦 trough
📦 vfile
📦 vfile-message
📦 remark-stringify
📦 mdast-util-to-markdown
📦 longest-streak
📦 unist-util-visit
📦 unist-util-is
📦 unist-util-visit-parents
📦 zwitch
📦 @milkdown/design-system

🔨 Add the following dependencies to your serverDependenciesToBundle

"@milkdown/core",
"@milkdown/ctx",
"@milkdown/design-system",
"@milkdown/exception",
"@milkdown/transformer",
"bail",
"character-entities",
"decode-named-character-reference",
"longest-streak",
"mdast-util-from-markdown",
"mdast-util-to-markdown",
"mdast-util-to-string",
"micromark",
"micromark-core-commonmark",
"micromark-factory-destination",
"micromark-factory-label",
"micromark-factory-space",
"micromark-factory-title",
"micromark-factory-whitespace",
"micromark-util-character",
"micromark-util-chunked",
"micromark-util-classify-character",
"micromark-util-combine-extensions",
"micromark-util-decode-numeric-character-reference",
"micromark-util-decode-string",
"micromark-util-encode",
"micromark-util-html-tag-name",
"micromark-util-normalize-identifier",
"micromark-util-resolve-all",
"micromark-util-sanitize-uri",
"micromark-util-subtokenize",
"micromark-util-symbol",
"micromark-util-types",
"remark",
"remark-parse",
"remark-stringify",
"trough",
"unified",
"unist-util-is",
"unist-util-stringify-position",
"unist-util-visit",
"unist-util-visit-parents",
"vfile",
"vfile-message",
"zwitch"

chanpod commented 1 year ago

I'm running into this now. My serverless file was just kind of bundling the entire node_modules but apparently I did something that made that too big. I'm pretty sure the real bundle size is less than 5MB but hard to say since I can't get it to build properly. Currently I'm having to tediously build -> upload -> run app -> check error logs for missing dependency.

The script has been useful for some libraries but there's quite a few outliers that it doesn't catch. Any potential updates or improvements coming to make this easier?

dtinth commented 1 year ago

ESM is the future, not the present. Until then, the workaround I’m using is to bundle the libraries I use in the Remix app, along with all the transitive dependencies, into a single-file CommonJS module (I use tsup to generate such bundle), so that they can be correctly loaded into a Remix app until they fix this gotcha.

I hope Remix team fixes this issue soon. I spent many hours scratching my head over this and I was so close to migrating the app to Astro (which does not have any issue with ESM dependencies).

mind0bender commented 1 year ago

I'm getting yargs-parser is possibly an ESM only package and should be bundled with "serverDependenciesToBundle" in remix.config.js.

even though I'm using it in an action.

Am I doing something wrong?

armandabric commented 1 year ago

It seems this is fixed by https://github.com/remix-run/remix/pull/6916

@pcattori Can your confirm?

pcattori commented 4 months ago

It seems this is fixed by #6916

@pcattori Can your confirm?

Yes, plus serverDependenciesToBundle is no longer a supported API with Remix Vite. You should instead use Vite's ssr.noExternal option as described in the ESM/CJS troubleshooting docs

remix-run / remix