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

issue with ApolloProvider types on monorepo #1928

Closed
hemedani opened this issue Jul 25, 2019 · 6 comments · Fixed by #1931
Closed

issue with ApolloProvider types on monorepo #1928

hemedani opened this issue Jul 25, 2019 · 6 comments · Fixed by #1931

Comments

@hemedani
Copy link

hemedani commented Jul 25, 2019

pnpm version:

 [I] ➜ pnpm -v
3.6.0

Code to reproduce the issue:

ReactDOM.render(
  <ApolloProvider client={client}>
    <App />
  </ApolloProvider>,
  document.getElementById("root")
);

the actual file address on git repo is this : ./packages/sefood-tracing-fr/src/index.ts

the issue is it as say typescript :

JSX element type 'ApolloProvider<unknown>' is not a constructor function for JSX elements.
  Type 'ApolloProvider<unknown>' is missing the following properties from type 'ElementClass': context, setState, forceUpdate, props, and 2 more.ts(2605)

Expected behavior:

detect the true type for ApolloProvider

Actual behavior:

Additional information:

I create a github repo to see the error in action

  • node -v prints: v8.11.2
  • Windows, OS X, or Linux?: OS X
@zkochan
Copy link
Member

zkochan commented Jul 27, 2019

This is probably some bug with how pnpm creates the flat node_modules

@zkochan
Copy link
Member

zkochan commented Jul 27, 2019

The issue is that currently shamefully-flatten does not work correctly with shared-workspace-lockfile (we'll need to add some error message to prevent confusion).

I have found 2 solutions for your problem:

1. fixing the dependencies of apollo-client

There are a few issues with apollo-client because it misses some deps.

Create the following pnpmfile.js in the root of your monorepo:

module.exports = {
    hooks: {
        readPackage (manifest) {
            switch (manifest.name) {
                case 'react-apollo':
                    manifest.dependencies['apollo-client'] = '^2.6.3' // taken from apollo-boost
                    manifest.peerDependencies['@types/react'] = '*'
                    break
            }
            return manifest
        }
    }
}

Run rm -rf node_modules pnpm-lock.yaml && pnpm m i. Issue solved

2. Don't use shared-workspace-lockfile

Add shared-workspace-lockfile = false to the .npmrc in the root of your monorepo. Remove pnpm-lock.yaml and node_modules from the root of your monorepo. Run pnpm m i. Issue solved.

@hemedani
Copy link
Author

two solution working perfectly fine. which one is better

@zkochan
Copy link
Member

zkochan commented Jul 28, 2019 via email

@zkochan
Copy link
Member

zkochan commented Jul 28, 2019

Seems like there's a 3rd solution and we can use it for a fix in pnpm. You can create an empty package.json in the root of the monorepo. no, this one doesn't work consistently

@zkochan zkochan added size: M and removed size: XL labels Jul 28, 2019
@zkochan zkochan added this to the v3.6 milestone Jul 28, 2019
zkochan added a commit that referenced this issue Jul 29, 2019
Flat node_modules is only allowed in the workspace root.

shamefully-flatten should only hoists dependencies of the
root workspace package.

close #1928
PR #1931

BREAKING CHANGE: @pnpm/shamefully-flatten
hwillson pushed a commit to apollographql/react-apollo that referenced this issue Aug 13, 2019
* Add an optional peer dependency to react-apollo

`@types/react` should be an optional peer dependency of react-apollo.

Related issue pnpm/pnpm#1928

* Add `@types/react` peer dep to all entry points

* Remove support for yarn only features

* Changelog update
@VinceOPS
Copy link

VinceOPS commented Oct 31, 2022

Hi @zkochan! Sorry for raising this (old) issue. I'm encountering the exact same problem with pnpm 7.14.0 and workspaces.

With both ink-table (using react ink), on Table component, and nextjs (Head component).

I tried the "solution 1" you gave here (using .pnpmfile.cjs), but I'm still having this error from tsc.

Did I miss anything? Ready to try something else, if it can help diagnosing the issue.

Thanks!


EDIT - I understood the problem. It happes when a package has declared @types/react as a dev dependency but not as a peer dependency.
Solve it with .pnpmfile.cjs based on your "solution 1":

const readPackage = (package, context) => pipe(
  fixPnpmModulesFlattening(context),
)(package);

module.exports = {
  hooks: {
    readPackage
  }
}

const composerReducer = (f, g) => (arg) => g(f(arg));
const pipe = (...fns) => fns.reduce(composerReducer);

const fixPnpmModulesFlattening = (context) => (package) => {
  const addMissingPeerDependenciesTo = copyDevDependencyToPeerDependencies(context);
  return pipe(
    addMissingPeerDependenciesTo('@types/react', ['next', 'ink-table']),
    addMissingPeerDependenciesTo('ink', ['ink-testing-library']),
  )(package);
}

const copyDevDependencyToPeerDependencies = (context) => (dependencyName, targetPackages) => (currentPackage) => {
  if (
    !targetPackages.includes(currentPackage.name) ||
    !currentPackage.devDependencies ||
    !currentPackage.devDependencies[dependencyName] ||
    (currentPackage.peerDependencies && currentPackage.peerDependencies[dependencyName])
  ) {
    return currentPackage;
  }

  context.log(`[pnpm fix] Add ${dependencyName} as a peer dependency of ${currentPackage.name}`);
  currentPackage.peerDependencies = {
    ...currentPackage.peerDependencies,
    [dependencyName]: '*', // currentPackage.devDependencies[dependencyName],
  };
  currentPackage.peerDependenciesMeta = { 
    ...currentPackage.peerDependenciesMeta,
    [dependencyName]: { optional: true },
  };

  return currentPackage;
}

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

Successfully merging a pull request may close this issue.

3 participants