Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using pnpm with remix package #154

Closed
mjackson opened this issue May 11, 2021 · 16 comments
Closed

Using pnpm with remix package #154

mjackson opened this issue May 11, 2021 · 16 comments
Labels
enhancement New feature or request

Comments

@mjackson
Copy link
Member

The remix package is a special one because its exports change based on what other packages you have installed. However, instead of resolving packages at runtime, pnpm uses hard links on the file system at install time.

What this means for us is that when you install remix with pnpm, it is linked into a node_modules/.pnpm/remix@version directory. Inside that, there is another node_modules directory that contains the remix package and is supposed to contain links to its dependencies as well. But they aren't there since remix doesn't explicitly list its dependencies. This means that when remix imports something, it has to go look for it in node_modules.

I'm not sure exactly why this is a problem for pnpm, but I believe it ends up causing two different copies of Remix to be loaded in the browser so we don't get the same context objects and <Outlet> renders null. Still need to look into this further.

@mjackson mjackson added the enhancement New feature or request label May 11, 2021
@tylerbrostrom
Copy link
Contributor

tylerbrostrom commented Nov 24, 2021

pnpm user here. This issue seems to be resolved… but don’t quote me on that.

The following works…

  1. pnpm init remix@latest
  2. Follow CLI prompts
    • Select “Remix App Server”
    • Select “Typescript”
    • Decline automatic running of npm install (i.e. type “n”)
  3. Change into project directory and run pnpm install
  4. Run pnpm postinstall (by default, pnpm does not automatically run pre and post script hooks)
  5. Run pnpm dev
Tool Version
node v16.13.0
pnpm 6.22.2
remix 1.0.5

And as far as I can see…

  • Demo app works as expected (<Outlet /> renders as expected).
  • Dependencies are resolved in the browser (and by the TypeScript language server).

@itsMapleLeaf
Copy link
Contributor

One helpful step would be to configure shamefully-hoist = true in .npmrc. Remix may rely on some packages that might not be declared in dependencies, and this flag hoists all nested dependencies so they're accessible.

@tylerbrostrom
Copy link
Contributor

One helpful step would be to configure shamefully-hoist = true in .npmrc.

Thanks. I did not have to do that. The default demo app just works.

@itsMapleLeaf
Copy link
Contributor

itsMapleLeaf commented Nov 24, 2021

I did 😅 I got a typescript error trying to do import type { LoaderFunction } from 'remix' (or some other type), which is a part of @remix-run/server-runtime and wasn't installed by default

Though you could argue that it should be 🤷

@matthewlynch
Copy link

matthewlynch commented Nov 24, 2021

pnpm is working for me following the steps @tylerbrostrom mentioned above.

I couldn't get the site to start in dev mode when it was a package inside of a workspace though due to this error:

> ../node_modules/.pnpm/@remix-run+react@1.0.5_react-dom@17.0.2+react@17.0.2/node_modules/@remix-run/react/browser/server.js:11:9: error: No matching export in "../node_modules/.pnpm/history@4.10.1/node_modules/history/esm/history.js" for import "Action"
    11 │ import { Action, createPath } from 'history';
       ╵          ~~~~~~

Versions:
node: 14.17.4
pnpm: 6.23.2

@raymclee
Copy link

shamefully-hoist = true

same here using pnpm workspace

@eldarshamukhamedov
Copy link
Contributor

Hey @matthewlynch , did @tylerbrostrom's steps help you bypass the "No matching export" error? I'm assuming it's related to PNPM hoisting.

I'm working in a RushJS monorepo which uses PNPM under the hood with a shared PNPM cache, and running into the same issue. I wonder if there's a missing history peer dep somewhere in the Remix stack? 🤔

@eldarshamukhamedov
Copy link
Contributor

eldarshamukhamedov commented Nov 28, 2021

@matthewlynch Okay, I think I figured it out. PNPM is very picky/explicit about dependencies, which helps to avoid "phantom dependencies". The @remix-run/react package uses history directly here:
https://github.com/remix-run/remix/blob/main/packages/remix-react/browser.tsx#L1-L2

But history is not explicitly listed as a dependency of @remix-run/react here:
https://github.com/remix-run/remix/blob/main/packages/remix-react/package.json#L15-L17

While NPM and Yarn will happily resolve the history phantom dependency via the react-router-dom dependency, thanks to their filesystem-based/Node-like lookup mechanism, PNPM will not because of it's caching and symlinking approach.

See more on phantom dependencies in the RushJS docs, although this issue isn't limited to Rush and affects all PNPM monorepo/workspace setups.

Temporary resolution for me was to add a PNPM module resolution hook in a .pnpmfile.cjs:

module.exports = {
  hooks: {
    readPackage: (packageJson, context) => {
      if (packageJson.name === '@remix-run/react') {
        packageJson.dependencies = {
          ...packageJson.dependencies,
          history: '^5.1.0',
        };
      }
      return packageJson;
    },
  },
};

Long-term fix would be to update the @remix-run/react package to explicitly list the history dependency in its package.json file. @mjackson happy to open a PR if this sounds reasonable Went ahead and opened a PR #739 🙂

@itsMapleLeaf
Copy link
Contributor

Also: tools like depcheck can be used to see if there are imported modules that aren't listed in your deps. This is also important for Yarn PnP projects

@matthewlynch
Copy link

@eldarshamukhamedov your workaround solved my problem, thank you!

@lzm0x219
Copy link

lzm0x219 commented Dec 5, 2021

mark.

@lukejagodzinski
Copy link

lukejagodzinski commented Feb 7, 2022

I have the same error when using Remix with Yarn workspaces. I'm solving error after the error but this one I have no way of solving. It's basically makes Remix not usable in monorepo...

@lzm0x219
Copy link

lzm0x219 commented Apr 4, 2022

Is there any progress?

@eldarshamukhamedov
Copy link
Contributor

@0x219 have you tried upgrading to the latest version of Remix? I am running 1.3.4 in a Rush monorepo using PNPM and no longer need the workaround described above. I am not using PNPM workspaces, however, so your mileage may vary. I also ended up switching away from import from 'remix' to import from '@remix/node' (etc), since the postinstall approach is getting phased out, as far as I can tell.

@lzm0x219
Copy link

lzm0x219 commented Apr 6, 2022

@0x219 have you tried upgrading to the latest version of Remix? I am running 1.3.4 in a Rush monorepo using PNPM and no longer need the workaround described above. I am not using PNPM workspaces, however, so your mileage may vary. I also ended up switching away from import from 'remix' to import from '@remix/node' (etc), since the postinstall approach is getting phased out, as far as I can tell.

ok thanks, i get it.

@chaance
Copy link
Collaborator

chaance commented Apr 19, 2022

The fix for this moving forward will be to import modules directly from the correct scoped package rather than the remix package. See v1.4.0 release notes for details.

@chaance chaance closed this as completed Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

9 participants