[Bug]: Yarn 3 support? - Githubissues

azat-io commented 2 years ago

Which Remix packages are impacted?

remix (Remix core)
create-remix

What version of Remix are you using?

1.0.5

Steps to Reproduce

Trying to install dependencies with Yarn berry. My current Yarn version is v3.2.0-rc.5.

Log file:

# This file contains the result of Yarn building a package (azat-io@workspace:.)
# Script name: postinstall

(node:67592) ExperimentalWarning: --experimental-loader is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
EROFS: read-only filesystem, open '/node_modules/remix/package.json'

Dependencies in my package.json:

{
  "dependencies": {
    "@remix-run/node": "1.0.5",
    "@remix-run/react": "1.0.5",
    "@remix-run/serve": "1.0.5",
    "@remix-run/server-runtime": "1.0.5",
    "react": "17.0.2",
    "react-dom": "17.0.2",
    "remix": "1.0.5"
  },
  "devDependencies": {
    "@remix-run/dev": "1.0.5",
    "@types/react": "17.0.24",
    "@types/react-dom": "17.0.9",
    "typescript": "4.1.2"
  }
}

Expected Behavior

Installation successful

Actual Behavior

Installation failed

mxro commented 2 years ago

I am able to install on yarn v 3.0.2 but when running yarn build using remix@1.0.5 I get the following error:

Building Remix app in production mode...

 > error: [plugin: remix-mdx] Cannot find package 'xdm' imported from C:\Users\Max\repos\goldstack-mega\.yarn\cache\@remix-run-dev-npm-1.0.6-73f0b55172-2fca586246.zip\node_modules\@remix-run\dev\compiler\plugins\mdx.js
Did you mean to import xdm-npm-2.1.0-e825e62593-5a3f2434a5.zip/node_modules/xdm/index.js?

Build failed with 1 error:
error: Cannot find package 'xdm' imported from C:\Users\Max\repos\goldstack-mega\.yarn\cache\@remix-run-dev-npm-1.0.6-73f0b55172-2fca586246.zip\node_modules\@remix-run\dev\compiler\plugins\mdx.js
Did you mean to import xdm-npm-2.1.0-e825e62593-5a3f2434a5.zip/node_modules/xdm/index.js?
Built in 170ms

Assuming this may also be related to module resolution using yarn 2?

jacob-ebey commented 2 years ago

What node version are you on?

mxro commented 2 years ago

I'm using node v14.17.6

mxro commented 2 years ago

Also getting the following error when trying to run yarn postinstall:

$ yarn postinstall
Missing the "remix" package. Please run `npm install remix` before `remix setup`.
Successfully setup Remix for node.

But definitely did an install before running this!

jacob-ebey commented 2 years ago

Seems like a linking issue. I don't have time to play with Yarn3 right now but if you would like to investigate and report back that would be awesome. We generally recommend to use NPM as that's what we test against, but we also use yarn1 without issue.

mxro commented 2 years ago

Okay, thanks for having had a quick look into it and understand if this cannot be looked at further. Wouldn't really know where to start, but will check in again in a few months if support for Yarn 2/3/'Berry' would be provided by then.

rhefner commented 2 years ago

Just an FYI - I got remix to work a while back (as in March/April) with Yarn v2 PnP by patching @remix-run/dev:

diff --git a/compiler/assets.js b/compiler/assets.js
index c161dd31793837b2ce722ddbaa496f3d9c6b2bac..ef5989a66f30941bdb9cc884e70f62ec346c231b 100644
--- a/compiler/assets.js
+++ b/compiler/assets.js
@@ -30,7 +30,7 @@ async function createAssetsManifest(config, metafile) {
   for (let key of Object.keys(metafile.outputs).sort()) {
     let output = metafile.outputs[key];
     if (!output.entryPoint) continue;
-    let entryPointFile = path.resolve(output.entryPoint.replace(/(^browser-route-module:|\?browser$)/g, ""));
+    let entryPointFile = path.resolve(output.entryPoint.replace(/(^pnp:|^browser-route-module:|\?browser$)/g, ""));

     if (entryPointFile === entryClientFile) {
       entry = {
diff --git a/compiler.js b/compiler.js
index 86e2c589195f8e27560696891c09583d32d09e63..e79e849cd877ab10ced733f01a517bdf14aaabb9 100644
--- a/compiler.js
+++ b/compiler.js
@@ -9,6 +9,7 @@ var module$1 = require('module');
 var esbuild = require('esbuild');
 var debounce = require('lodash.debounce');
 var chokidar = require('chokidar');
+var { pnpPlugin } = require('@yarnpkg/esbuild-plugin-pnp')
 var build$1 = require('./build.js');
 var config = require('./config.js');
 var invariant = require('./invariant.js');
@@ -194,7 +195,7 @@ async function createBrowserBuild(config, options) {
     define: {
       "process.env.NODE_ENV": JSON.stringify(options.mode)
     },
-    plugins: [browserRouteModulesPlugin(config, /\?browser$/), emptyModulesPlugin(config, /\.server(\.[jt]sx?)?$/)]
+    plugins: [browserRouteModulesPlugin(config, /\?browser$/), emptyModulesPlugin(config, /\.server(\.[jt]sx?)?$/), pnpPlugin()]
   });
 }

@@ -234,7 +235,7 @@ async function createServerBuild(config, options) {
       }

       return false;
-    })]
+    }), pnpPlugin()]
   });
 }

The patch above of course won't work with the latest code, but back then it was possible to get it to work with PnP.. Remix does work with Yarn v3 if you're using nodeLinker: node-modules - If you're in a monorepo, you'll likely need to add the following to the Remix packages' package.json as well:

"installConfig": {
  "hoistingLimits": "workspaces"
}

b/c Remix gets hoisted to the top-level node_modules/:

Watching Remix app in development mode...

 > ../../node_modules/remix/browser/index.js:11:14: error: Could not resolve "./client"
    11 │ export * from './client';
       ╵               ~~~~~~~~~~

  > ../../node_modules/remix/browser/index.js:12:14: error: Could not resolve "./server"
    12 │ export * from './server';
       ╵               ~~~~~~~~~~

  > ../../node_modules/remix/browser/index.js:13:14: error: Could not resolve "./platform"
    13 │ export * from './platform';
       ╵               ~~~~~~~~~~~~

Build failed with 3 errors:
../../node_modules/remix/browser/index.js:11:14: error: Could not resolve "./client"
../../node_modules/remix/browser/index.js:12:14: error: Could not resolve "./server"
../../node_modules/remix/browser/index.js:13:14: error: Could not resolve "./platform"
💿 Built in 224ms

Without hoisting (using the hoistingLimits above), it at least spins up properly:

$ yarn workspace remix-app-template run dev
Watching Remix app in development mode...
💿 Built in 220ms
Remix App Server started at http://localhost:3000
GET / 200 - - 20.309 ms

If you switch back/forth between the hoisting rules above, you should clear out node_modules/ as some things seem to stay around with at least Yarn v3.0.1:

find . -name "node_modules" -type d -prune -exec rm -rf '{}' +

mxro commented 2 years ago

Thanks @rhefner - looks like it won't be too complicated to make remix work with Yarn 2/3 👍

mihaa1 commented 2 years ago

Experiencing the same issue with yarn 3.1.1 + corepack. Just ran the tutorial:

npx create-remix@latest
posinstall script fails with: @remix-run/dev tried to access @remix-run/node, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound

rhefner commented 2 years ago

@mihaa1 Don't know anything about corepack, but if I'm not mistaken, that error is coming from Yarn. Have you tried updating packageExtensions in .yarnrc.yml?

For example:

packageExtensions:
  "@remix-run/dev@*":
    dependencies:
      "@remix-run/node": "*"

silouanwright commented 2 years ago

I can also confirm that the install does not work with Yarn 2+ Plug N Play setup. The issue is mostly around the postinstall command. Here is the first error triggered:

# This file contains the result of Yarn building a package (jokes@workspace:.)
# Script name: postinstall

@remix-run/dev tried to access @remix-run/server-runtime, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: @remix-run/server-runtime (via "@remix-run/server-runtime/package.json")
Required by: @remix-run/dev@npm:1.0.6 (via /Users/reydev/repos/jokes/.yarn/cache/@remix-run-dev-npm-1.0.6-73f0b55172-2fca586246.zip/node_modules/@remix-run/dev/)

Require stack:
- /Users/reydev/repos/jokes/.yarn/cache/@remix-run-dev-npm-1.0.6-73f0b55172-2fca586246.zip/node_modules/@remix-run/dev/setup.js
- /Users/reydev/repos/jokes/.yarn/cache/@remix-run-dev-npm-1.0.6-73f0b55172-2fca586246.zip/node_modules/@remix-run/dev/cli/commands.js
- /Users/reydev/repos/jokes/.yarn/cache/@remix-run-dev-npm-1.0.6-73f0b55172-2fca586246.zip/node_modules/@remix-run/dev/cli.js

When I then installed @remix-run/server-runtime, this is the error I received:

# This file contains the result of Yarn building a package (jokes@workspace:.)
# Script name: postinstall

EROFS: read-only filesystem, open '/node_modules/remix/package.json'

The escape hatch of using nodeLinker technically works BUT I would advocate that this isn't really adding support for Yarn 2+, as this disables Plug N Play, which by default is enabled on new projects bootstrapped for Yarn 2+ (yarn init -2) and is forcing a pretty big architectural decision on a team for their entire project, that actually has some concrete benefits: https://yarnpkg.com/features/pnp. I would prefer just saying "we don't support Yarn 2+ than going the route that has us recommending us opt out of its biggest feature.

cmd-johnson commented 2 years ago

I dug a bit deeper into what it would take to support yarn PnP, and these are my findings so far:

TLDR: you're better off sticking to the node-modules linker for now.

remix performs some dark magic with the remix package in the postinstall hook that will never be compatible with yarn PnP, and frankly is an anti-pattern (don't overwrite files in packages, please). I ended up throwing out remix as a dependency and now import everything directly from the @remix-run/... packages as required
there's a few dependency/peer dependency declarations missing in the remix packages. which is an easy fix.
after fixing the dependency declarations, you'll run into a bug in the current version of yarn, which appears to be fixed in the current canary build of yarn (yarn-3.2.0-rc.8, install it using yarn set version canary). (ref. yarnpkg/berry/issues/3687)
now we run into an issue with esbuild: to make React available in all source files without having to explicitly import it, remix uses the inject option to add the contents of a react.ts file to every compiled source file. While it is possible to read files inside the zipped pnp dependencies wihle running in node, esbuild tries to read those files from an executable written in go, which fails. I added some code to compiler.ts in @remix-run/dev to copy the react.ts file contents to a temporary build folder and include that file instead. Now the build goes through.
however, we're still not quite there yet. There's a server listening on 8002 now, but it only returns 426 Upgrade Required without any indications of what protocol it expects instead of HTTP/1.1. Looks like it doesn't start up properly, because the "Remix App Server started at ..." never gets logged to the console.

For reference, this is my current .yarnrc.yml, and these are the changes I made to the compiler.ts file to get esbuild to work

That's where I'm at now, I'll continue digging once I find some more time.

cmd-johnson commented 2 years ago

I made some more progress! The build now runs fine with PnP and the dev server starts up correctly as well. To test my changes:

Clone and build my changed version of remix:

git clone https://github.com/cmd-johnson/remix/tree/feature/yarn-pnp
cd remix
yarn install
yarn build

In the project you want to use yarn PnP in:

switch to the current canary build of yarn: yarn set version canary. Yarn < 3.2.0 has a bug preventing remix to work with PnP.
remove the remix dependency from your package.json, it just doesn't work with PnP right now (I'll see what I can do about that next)
while you're at it, remove remix setup node from your postinstall script. It will fail when using PnP

add a resolutions entry to your package.json:

  "resolutions": {
    "@remix-run/dev": "file:///path/to/the/cloned/remix/project/build/node_modules/@remix-run/dev",
    "@remix-run/express": "file:///path/to/the/cloned/remix/project/build/node_modules/@remix-run/express",
    "@remix-run/node": "file:///path/to/the/cloned/remix/project/build/node_modules/@remix-run/node",
    "@remix-run/react": "file:///path/to/the/cloned/remix/project/build/node_modules/@remix-run/react",
    "@remix-run/serve": "file:///path/to/the/cloned/remix/project/build/node_modules/@remix-run/serve"
  },

This makes sure you're using the patched @remix-run/* versions instead of the original ones throughout your whole project

run yarn add @yarnpkg/esbuild-plugin-pnp. This is an optional peer dependency I've added to @remix-run/dev that you'll need when using PnP for the esbuild build to go through
run yarn add express. @remix-run/serve depends on it as a peer dependency. Depending on your setup, you might be able to get away with adding express as a peer dependency instead
the last thing to change is fixing all imports from the remix package to imports from @remix-run/* packages. E.g.:
- RemixBrowser → @remix-run/react/browser
- EntryContext, MetaFunction → @remix-run/server-runtime (you might have to add a dependency to @remix-run/server-runtime to your project as well)
- Most other dependencies appear to be exported by @remix-run/react

After that, you should be able to start the dev server using yarn dev as usual.

I hope I didn't forget anything. I'll draft a PR once I get around to get the remix package to work with PnP.

drewdecarme commented 2 years ago

Very interested in seeing if we can't find a solution to this soon. I'm a prolific YarnPnP user and Remix feels so right to me. Really excited to start using both together!

SleeplessByte commented 2 years ago

I also ran into issues when I tried to upgrade yarn because yarn install --production (the equivalent of npm prune --production) leads to remix missing in .bin . This is due to a longstanding issue (see: https://github.com/yarnpkg/yarn/issues/761#issuecomment-273052409).

When remix works correctly with yarn 2/3, I recommend also making sure the following works:

yarn set version stable
yarn plugin import workspace-tools
yarn install

# This is the Yarn 3 replacement for yarn install --production
yarn workspaces focus --production

tavoyne commented 2 years ago

Any progress on that, @cmd-johnson? Can give you a hand if needed.

cmd-johnson commented 2 years ago

@tavoyne #1316 should be ready for merging, but feel free to test it yourself! Everything should work as long as you don't import anything from the remix package and use the @remix-run/* packages directly instead. Also check out the README from the yarn example I added in the PR: https://github.com/remix-run/remix/blob/7af1ed35603839d85866d3d793ecc89c2244236b/examples/yarn-pnp/README.md

penx commented 2 years ago

https://github.com/remix-run/remix/pull/2359 has been merged and included in 1.3.4 which at least solves the basic setup for me.

I was able to get the remix "basic" template working with 1.3.4 in a yarn 3 monorepo (using node_modules resolution) by:

Removing "postinstall": "remix setup node", from package.json before doing any install
Changing root.tsx imports to:

import {
  Links,
  LiveReload,
  Meta,
  Outlet,
  Scripts,
  ScrollRestoration,
} from "@remix-run/react";
import type { MetaFunction } from "@remix-run/node";

Changing the entry.server.tsx imports to:

import { RemixServer } from "@remix-run/react";
import type { EntryContext } from "@remix-run/node";

Changing entry.client.ts imports to:

import { RemixBrowser } from "@remix-run/react";

lensbart commented 2 years ago

Somewhat related: when running yarn dlx create-remix@latest instead of npx create-remix@latest, you get the following error:

Error: @remix-run/dev tried to access .bin, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

yarn dlx is the idiomatic way to run packages without installing when using Yarn, and is analogous to npx.

The same error appears when running yarn dev.

chaance commented 2 years ago

Our guidance here is to get rid of all imports from remix in favor of importing directly from our scoped modules, and this should work with Yarn 3. We now provide a handy migrate script to make this quick and easy! See release notes for 1.4.0 for details.

lensbart commented 2 years ago

@chaance This is has not been fixed — I still get an error when running yarn dlx create-remix@latest:

/private/var/folders/dk/h4tc0zx90g9fdr9pbw48k6pr0000gn/T/xfs-a2f22fa1/dlx-26512/.pnp.cjs:16327
      Error.captureStackTrace(firstError);
            ^

Error: @remix-run/dev tried to access .bin, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: .bin (via ".bin/jscodeshift")
Required by: @remix-run/dev@npm:1.4.1 (via /Users/bart/.yarn/berry/cache/@remix-run-dev-npm-1.4.1-2ab50c1d1f-8.zip/node_modules/@remix-run/dev/cli/migrate/)

Require stack:
- /Users/bart/.yarn/berry/cache/@remix-run-dev-npm-1.4.1-2ab50c1d1f-8.zip/node_modules/@remix-run/dev/cli/migrate/jscodeshift.js
- /Users/bart/.yarn/berry/cache/@remix-run-dev-npm-1.4.1-2ab50c1d1f-8.zip/node_modules/@remix-run/dev/cli/migrate/migrations/replace-remix-imports/index.js
- /Users/bart/.yarn/berry/cache/@remix-run-dev-npm-1.4.1-2ab50c1d1f-8.zip/node_modules/@remix-run/dev/cli/commands.js
- /Users/bart/.yarn/berry/cache/@remix-run-dev-npm-1.4.1-2ab50c1d1f-8.zip/node_modules/@remix-run/dev/cli/run.js
- /Users/bart/.yarn/berry/cache/@remix-run-dev-npm-1.4.1-2ab50c1d1f-8.zip/node_modules/@remix-run/dev/cli/index.js
- /Users/bart/.yarn/berry/cache/@remix-run-dev-npm-1.4.1-2ab50c1d1f-8.zip/node_modules/@remix-run/dev/index.js
- /Users/bart/.yarn/berry/cache/create-remix-npm-1.4.1-9c74da98d5-8.zip/node_modules/create-remix/cli.js
    at Function.require$$0.Module._resolveFilename (/private/var/folders/dk/h4tc0zx90g9fdr9pbw48k6pr0000gn/T/xfs-a2f22fa1/dlx-26512/.pnp.cjs:16327:13)
    at Function.resolve (node:internal/modules/cjs/helpers:108:19)
    at Object.<anonymous> (/Users/bart/.yarn/berry/cache/@remix-run-dev-npm-1.4.1-2ab50c1d1f-8.zip/node_modules/@remix-run/dev/cli/migrate/jscodeshift.js:17:39)
    at Module._compile (node:internal/modules/cjs/loader:1103:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1155:10)
    at Object.require$$0.Module._extensions..js (/private/var/folders/dk/h4tc0zx90g9fdr9pbw48k6pr0000gn/T/xfs-a2f22fa1/dlx-26512/.pnp.cjs:16371:33)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.require$$0.Module._load (/private/var/folders/dk/h4tc0zx90g9fdr9pbw48k6pr0000gn/T/xfs-a2f22fa1/dlx-26512/.pnp.cjs:16211:14)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)

This may be solved by using require.resolve("jscodeshift/bin/jscodeshift") instead, as mentioned here. That PR may include other fixes required for Yarn PNP.

Same error when using npx create-remix@latest to get around this error and then running yarn dev.

chaance commented 2 years ago

@lensbart Thanks for flagging, we'll take a look 👍

eric-burel commented 2 years ago

Hi, I am working on a Remix stack which happens to live in a Yarn 3 monorepo: https://github.com/VulcanJS/vulcan-npm/pull/116 I'll detail my issues and learnings in the PR as I go by

wladpaiva commented 2 years ago

I got both Yarn 3 and monorepo working just by adding the nodeLinker: node-modules in .yarnrc.yml and adding serverDependenciesToBundle: [/.*/] in remix.config.js

https://github.com/wladiston/remix-vercel-monorepo

MichaelDeBoey commented 2 years ago

@lensbart Your problem with jscodeshift will be fixed by #3114

lensbart commented 2 years ago

Thank you, @MichaelDeBoey. yarn dlx create-remix@v0.0.0-nightly-34577c6-20220507 now runs without error in a Yarn 3 monorepository using PnP.

yarn dev still gives the following:

✘ [ERROR] Could not read from file: .yarn/cache/@remix-run-dev-npm-0.0.0-nightly-34577c6-20220507-bdbe7194e9-1f94a83917.zip/node_modules/@remix-run/dev/compiler/shims/react.ts

 ✘ [ERROR] Could not resolve "@remix-run/react"

    app/entry.client.tsx:1:29:
      1 │ import { RemixBrowser } from "@remix-run/react";
        ╵                              ~~~~~~~~~~~~~~~~~~

  You can mark the path "@remix-run/react" as external to exclude it from the bundle, which will remove this error.

 ✘ [ERROR] Could not resolve "react-dom"

    app/entry.client.tsx:2:24:
      2 │ import { hydrate } from "react-dom";
        ╵                         ~~~~~~~~~~~

  You can mark the path "react-dom" as external to exclude it from the bundle, which will remove this error.

 ✘ [ERROR] Could not resolve "@remix-run/react"

    app/root.tsx:9:7:
      9 │ } from "@remix-run/react";
        ╵        ~~~~~~~~~~~~~~~~~~

  You can mark the path "@remix-run/react" as external to exclude it from the bundle, which will remove this error.

Build failed with 4 errors:
error: Could not read from file: .yarn/cache/@remix-run-dev-npm-0.0.0-nightly-34577c6-20220507-bdbe7194e9-1f94a83917.zip/node_modules/@remix-run/dev/compiler/shims/react.ts
app/entry.client.tsx:1:29: ERROR: Could not resolve "@remix-run/react"
app/entry.client.tsx:2:24: ERROR: Could not resolve "react-dom"
app/root.tsx:9:7: ERROR: Could not resolve "@remix-run/react"
Error
    at Object.onBuildFailure (.yarn/cache/@remix-run-dev-npm-0.0.0-nightly-34577c6-20220507-bdbe7194e9-1f94a83917.zip/node_modules/@remix-run/dev/cli/commands.js:153:13)
    at buildEverything (.yarn/cache/@remix-run-dev-npm-0.0.0-nightly-34577c6-20220507-bdbe7194e9-1f94a83917.zip/node_modules/@remix-run/dev/compiler.js:280:13)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Object.build (.yarn/cache/@remix-run-dev-npm-0.0.0-nightly-34577c6-20220507-bdbe7194e9-1f94a83917.zip/node_modules/@remix-run/dev/compiler.js:105:3)
    at async Object.build (.yarn/cache/@remix-run-dev-npm-0.0.0-nightly-34577c6-20220507-bdbe7194e9-1f94a83917.zip/node_modules/@remix-run/dev/cli/commands.js:148:3)
    at async Object.run (.yarn/cache/@remix-run-dev-npm-0.0.0-nightly-34577c6-20220507-bdbe7194e9-1f94a83917.zip/node_modules/@remix-run/dev/cli/run.js:468:7)

Not sure how I can go about fixing this issue. I tried adding @remix-run/react and react-dom to serverDependenciesToBundle in remix.config.js, to no avail. And it doesn’t seem like an error that can be fixed by configuring packageExtensions in .yarnrc.yml either. I also tried adding "dependenciesMeta": { "@remix-run/react": { "built": false }, "react-dom": { "built": false } } to the monorepo’s root package.json, but that didn’t help either.

Thoughts?

ezracelli commented 2 years ago

Thank you, @MichaelDeBoey. yarn dlx create-remix@v0.0.0-nightly-34577c6-20220507 now runs without error in a Yarn 3 monorepository using PnP.

yarn dev still gives the following:

✘ [ERROR] Could not read from file: .yarn/cache/@remix-run-dev-npm-0.0.0-nightly-34577c6-20220507-bdbe7194e9-1f94a83917.zip/node_modules/@remix-run/dev/compiler/shims/react.ts

 ✘ [ERROR] Could not resolve "@remix-run/react"

    app/entry.client.tsx:1:29:
      1 │ import { RemixBrowser } from "@remix-run/react";
        ╵                              ~~~~~~~~~~~~~~~~~~

  You can mark the path "@remix-run/react" as external to exclude it from the bundle, which will remove this error.

 ✘ [ERROR] Could not resolve "react-dom"

    app/entry.client.tsx:2:24:
      2 │ import { hydrate } from "react-dom";
        ╵                         ~~~~~~~~~~~

  You can mark the path "react-dom" as external to exclude it from the bundle, which will remove this error.

 ✘ [ERROR] Could not resolve "@remix-run/react"

    app/root.tsx:9:7:
      9 │ } from "@remix-run/react";
        ╵        ~~~~~~~~~~~~~~~~~~

  You can mark the path "@remix-run/react" as external to exclude it from the bundle, which will remove this error.

Build failed with 4 errors:
error: Could not read from file: .yarn/cache/@remix-run-dev-npm-0.0.0-nightly-34577c6-20220507-bdbe7194e9-1f94a83917.zip/node_modules/@remix-run/dev/compiler/shims/react.ts
app/entry.client.tsx:1:29: ERROR: Could not resolve "@remix-run/react"
app/entry.client.tsx:2:24: ERROR: Could not resolve "react-dom"
app/root.tsx:9:7: ERROR: Could not resolve "@remix-run/react"
Error
    at Object.onBuildFailure (.yarn/cache/@remix-run-dev-npm-0.0.0-nightly-34577c6-20220507-bdbe7194e9-1f94a83917.zip/node_modules/@remix-run/dev/cli/commands.js:153:13)
    at buildEverything (.yarn/cache/@remix-run-dev-npm-0.0.0-nightly-34577c6-20220507-bdbe7194e9-1f94a83917.zip/node_modules/@remix-run/dev/compiler.js:280:13)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Object.build (.yarn/cache/@remix-run-dev-npm-0.0.0-nightly-34577c6-20220507-bdbe7194e9-1f94a83917.zip/node_modules/@remix-run/dev/compiler.js:105:3)
    at async Object.build (.yarn/cache/@remix-run-dev-npm-0.0.0-nightly-34577c6-20220507-bdbe7194e9-1f94a83917.zip/node_modules/@remix-run/dev/cli/commands.js:148:3)
    at async Object.run (.yarn/cache/@remix-run-dev-npm-0.0.0-nightly-34577c6-20220507-bdbe7194e9-1f94a83917.zip/node_modules/@remix-run/dev/cli/run.js:468:7)

Not sure how I can go about fixing this issue. I tried adding @remix-run/react and react-dom to serverDependenciesToBundle in remix.config.js, to no avail. And it doesn’t seem like an error that can be fixed by configuring packageExtensions in .yarnrc.yml either. I also tried adding "dependenciesMeta": { "@remix-run/react": { "built": false }, "react-dom": { "built": false } } to the monorepo’s root package.json, but that didn’t help either.

Thoughts?

I was experiencing this as well! I actually recognized this as esbuild (which remix uses under the hood) not supporting yarn's PnP resolver -- (closed) issue for that is here: https://github.com/evanw/esbuild/issues/154

To fix this, I'd normally use @yarnpkg/esbuild-plugin-pnp, but remix does not support adding esbuild plugins -- (closed) issue for that is here: https://github.com/remix-run/remix/issues/717. As a workaround, I used the extremely hacky override mentioned in that issue to add in @yarnpkg/esbuild-plugin-pnp to the compiler.

./esbuild.register.js:

const Module = require("node:module");

const { pnpPlugin } = require("@yarnpkg/esbuild-plugin-pnp");
const esbuild = require("esbuild");

const originalBuild = esbuild.build;
const build = (options) => {
    return originalBuild({
        ...options,
        plugins: [...options.plugins, pnpPlugin()],
    });
};

const originalRequire = Module.prototype.require;
Module.prototype.require = function (id) {
    if (id === "esbuild") {
        return { ...esbuild, build };
    }

    return originalRequire.apply(this, arguments);
};

package.json:

{
  "scripts": {
    "build": "NODE_OPTIONS=\"${NODE_OPTIONS} --require ./esbuild.register\" remix build",
    "start:dev": "NODE_OPTIONS=\"${NODE_OPTIONS} --require ./esbuild.register\" remix dev"
  }
}

The next issue that I ran into was this:

The following error is a bug in Remix; please open an issue! https://github.com/remix-run/remix/issues/new
Missing output for entry point
💿 Built in 453ms

Turns out, @yarnpkg/esbuild-plugin-pnp adds the pnp namespace to files loaded via PnP (so it knows how to process them later; this is common practice in the esbuild plugin world). The files it loads are emitted in the esbuild manifest with filenames looking like pnp:/path/to/filename.ext.

The issue is that to find the project's entry point, @remix-run/dev is checking for entryPointFile === entryClientFile. This would normally be fine, because it's an absolute path, but in this case, this check will be false for the correct file because of the pnp: prefix added by @yarnpkg/esbuild-plugin-pnp that's not normally there.

So, the last step was to yarn patch @remix-run/dev:

diff --git a/compiler/assets.js b/compiler/assets.js
index c675d328d512ceff6e8e6c9f07578f132417d375..d837842d85a07158857778b00003a4ca089bd370 100644
--- a/compiler/assets.js
+++ b/compiler/assets.js
@@ -61,7 +61,7 @@ async function createAssetsManifest(config, metafile, cssModules) {
     if (!output.entryPoint) continue;
     let entryPointFile = path__namespace.resolve(output.entryPoint.replace(/(^browser-route-module:|\?browser$)/g, ""));

-    if (entryPointFile === entryClientFile) {
+    if (entryPointFile.endsWith(entryClientFile)) {
       entry = {
         module: resolveUrl(key),
         imports: resolveImports(output.imports)

After this patch was created and applied, I was able to get remix working with yarn@4.0.0-rc.6! Ya boi loves to live on the bleeding edge 🚀

To the remix dev team, I'd really love to properly add PnP support to remix. It seems like it should be as simple as adding in @yarnpkg/esbuild-plugin-pnp to the default list of esbuild plugins (it's a no-op if there's no PnP API available) and applying a patch similar to the one above to @remix-run/dev. Happy to submit a PR if you're interested!~

cmd-johnson commented 2 years ago

@ezracelli That's basically what I did in #1316 (https://github.com/remix-run/remix/pull/1316/files#diff-ef8b0a35f03b499672160742c2d6b9688717e8d6b923dde0e10d2631ab0c09d6) I even declared the eslint pnp plugin as an optional dependency, so it doesn't even get loaded if you don't need it. Unfortunately the PR never went anywhere and is outdated again, but I'd happily rebase it if it gets PnP support to remix

eric-burel commented 2 years ago

Small issue I am hitting: having a package-lock.json will stress out lerna (or lerna-lite) "publish" function, yet it's necessary for the Indie stack github actions to work ok

Edit: ok I think I got it: in a monorepo you cannot have a yarn.lock at app level, because that would prevent hoisting completely. So I need a way to force reinstalling or generate an app-level yarn.lock, tricky issue...

Details of the error:

Run bahmutov/npm-install@v1
running npm-install GitHub Action
Error: ENOENT: no such file or directory, open '/home/runner/work/eurodance-stack/eurodance-stack/package-lock.json'
    at Object.openSync (node:fs:585:[3](https://github.com/VulcanJS/eurodance-stack/runs/6492076400?check_suite_focus=true#step:5:4))
    at Object.readFileSync (node:fs:[4](https://github.com/VulcanJS/eurodance-stack/runs/6492076400?check_suite_focus=true#step:5:5)[5](https://github.com/VulcanJS/eurodance-stack/runs/6492076400?check_suite_focus=true#step:5:6)3:35)
    at Function.module.exports.hasha.fromFileSync (/home/runner/work/_actions/bahmutov/npm-install/v1/dist/index.js:75[6](https://github.com/VulcanJS/eurodance-stack/runs/6492076400?check_suite_focus=true#step:5:7)[8](https://github.com/VulcanJS/eurodance-stack/runs/6492076400?check_suite_focus=true#step:5:9):54)
    at Object.installInOneFolder (/home/runner/work/_actions/bahmutov/npm-install/v1/dist/index.js:3024:26)
    at npmInstallAction (/home/runner/work/_actions/bahmutov/npm-install/v1/dist/index.js:3086:21)
    at Object.<anonymous> (/home/runner/work/_actions/bahmutov/npm-install/v1/dist/index.js:3116:3)
    at __webpack_require__ (/home/runner/work/_actions/bahmutov/npm-install/v1/dist/index.js:24:31)
    at startup (/home/runner/work/_actions/bahmutov/npm-install/v1/dist/index.js:43:1[9](https://github.com/VulcanJS/eurodance-stack/runs/6492076400?check_suite_focus=true#step:5:10))
    at /home/runner/work/_actions/bahmutov/npm-install/v1/dist/index.js:49:18
    at Object.<anonymous> (/home/runner/work/_actions/bahmutov/npm-install/v1/dist/index.js:52:[10](https://github.com/VulcanJS/eurodance-stack/runs/6492076400?check_suite_focus=true#step:5:11)) {
  errno: -2,
  syscall: 'open',
  code: 'ENOENT',
  path: '/home/runner/work/eurodance-stack/eurodance-stack/package-lock.json'

Is there a way to either force an npm install if no package-lock.json or to "fallback" to yarn elegantly?

lensbart commented 2 years ago

@lensbart Thanks for flagging, we'll take a look 👍

@chaance sorry to bother you with this (I guess you guys must be busy so shortly before the Remix conf) — but could you give the solution suggested above a look? It seems like a simple fix that would help many people.

Thank you! 😊

github-actions[bot] commented 2 years ago

🤖 Hello there,

We just published version v0.0.0-nightly-1e32bfa-20220622 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

MichaelDeBoey commented 2 years ago

Closed by #1316

lensbart commented 2 years ago

Thank you.

Using the nightly build, I get the following warnings upon running yarn:

➤ YN0002: │ @remix-run/dev@npm:0.0.0-nightly-1e32bfa-20220622 [a4e66] doesn't provide react (pfb8fe), requested by @remix-run/server-runtime
➤ YN0002: │ @remix-run/dev@npm:0.0.0-nightly-1e32bfa-20220622 [a4e66] doesn't provide react-dom (pff415), requested by @remix-run/server-runtime
➤ YN0002: │ @remix-run/node@npm:0.0.0-nightly-1e32bfa-20220622 doesn't provide react (pb3d4d), requested by @remix-run/server-runtime
➤ YN0002: │ @remix-run/node@npm:0.0.0-nightly-1e32bfa-20220622 doesn't provide react-dom (p1d69f), requested by @remix-run/server-runtime

These can be muted by adding the following to .yarnrc.yml:

packageExtensions:
  # awaiting fix: https://github.com/remix-run/remix/issues/683
  '@remix-run/server-runtime@*':
    peerDependenciesMeta:
      react:
        optional: true
      react-dom:
        optional: true

lensbart commented 2 years ago

I’m still having a couple of issues when running Remix with Yarn v3.2.1, apart from the warning mentioned above:

when running yarn dlx create-remix@0.0.0-nightly-1e32bfa-20220622:
- The CLI menu fails right before the install (e.g. Just the basics / Deno / TypeScript / Yes) with Command failed: yarn config get @remix-run:registry. This also happens in a new project (cd into an empty directory → yarn init -2)
- When I run yarn config get @remix-run:registry by myself, I get the following error:
- Looking at the code, I’m guessing that the command doesn’t throw for NPM, but does for Yarn, in case no private registry configuration could be found. So I think one solution might be to wrap these lines in a try...catch block.
When running yarn (or yarn install) myself, the project seems to install correctly, albeit with the warnings mentioned in the post above.
- I confirmed that all @remix-run-prefixed packages listed in yarn.lock are from the nightly
(deleted because this might have been due to my own setup)
There is also an inconvenience when running yarn dlx create-remix, where:
- Yarn doesn’t want to run this command in a folder without package.json (fair enough, solved by running yarn init -2)
- the files generated by remix must be in a deeper folder (./hello in the screenshot below), because the script won’t generate them inside of a non-empty folder
- ...but Yarn complains when running yarn install inside hello because it doesn’t have a yarn.lock file and its parent directory looks like a Yarn project, which makes the CLI warn for a misconfigured monorepository. So I think the best approach is to let create-remix generate an (empty) lockfile as well.

I’m happy to help where I can, e.g. by submitting a PR for issue 1 (try/catch), 2 (listing dependencies in package.json or indicating them as optional) and 3 (write yarn.lock if not present in the current working directory, allow default npm init/yarn init-files in nonempty cwd).

Thanks in advance for your insight and consideration!

skoging commented 2 years ago

@lensbart I think 3 is due to the ordering of the @yarnpkg/esbuild-plugin-pnp and @esbuild-plugins/node-modules-polyfill. If I remember correctly the node-modules-polyfill plugin must be after the PnP plugin. I had the same problem when trying this with Cloudflare Pages. I got most of the way there, but ran into issues with wrangler also not supporting PnP and ended up abandoning my attempt once more.

lensbart commented 2 years ago

Created a small PR for 1. and 2.

For issue 4, some more guidance would be welcome, to understand the current “happy path” and how my testing deviates from it. (Between parentheses: the script runs fine inside of a monorepo setup)

lensbart commented 2 years ago

@skoging that seems to have been fixed in https://github.com/remix-run/remix/pull/3579, but unfortunately doesn’t resolve the issue on my end.

lensbart commented 2 years ago

Aha! What worked for me was swapping out @yarnpkg/esbuild-plugin-pnp with @esbuild-plugins/node-resolve, which serves the same purpose (compatibility with Yarn PnP) but is from the same repository as @esbuild-plugins/node-modules-polyfill.

Apologies for the many messages/notifications

See https://github.com/remix-run/remix/pull/3594 & other yarn-related PR’s https://github.com/remix-run/remix/pull/3595 https://github.com/remix-run/remix/pull/3599

skoging commented 2 years ago

I managed to get it working without swapping to a different plugin, but #3579 wasn't enough.

this line adds the @esbuild-plugins/node-resolve plugin to the front of the plugin list, meaning it's before the @yarnpkg/esbuild-plugin-pnp plugin. Changing plugins.unshift(NodeModulesPolyfillPlugin()) to plugins.push(NodeModulesPolyfillPlugin()) resolved this issue, but I don't know what other consequences moving the node-resolve plugin to be last instead of the first has.

This only happens when serverPlatform is not "node". I'm getting this problem when using Cloudflare Pages.

alisd23 commented 2 years ago

@skoging That was my lazy fix (and wasn't a fix at all), so I wouldn't look into it. I'm reverting it (and fixing the actual problem I was seeing) in PR #3633.

I'm not sure re-ordering in this case is right, but there may be multiple issues going on here that we're all trying to fix separately. Are you getting the same errors as I mentioned in #3579? Or more like these errors?

eric-burel commented 2 years ago

If some googlers are specifically looking for Yarn workspaces support, I've created a separate discussion: https://github.com/remix-run/remix/discussions/3668

remix-run / remix

[Bug]: Yarn 3 support? #683

Which Remix packages are impacted?

What version of Remix are you using?

Steps to Reproduce

Expected Behavior

Actual Behavior