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

Angular: @nxext/ionic-angular application schematic broken with pnpm in fresh Nx monorepo #808

Open
troywweber7 opened this issue Sep 14, 2022 · 1 comment
Labels
bug Something isn't working

Comments

@troywweber7
Copy link

troywweber7 commented Sep 14, 2022

Describe the bug

In a freshly created pnpm + Nx monorepo, the @nxext/ionic-angular:application schematic fails to run. If I fix the errors preventing the schematic from running and then try to build, test, e2e, or lint, I get additional errors. I tracked down a number of packages which were depending on phantom dependencies, and if I address those phantom dependencies with a .pnpmfile.cjs, I'm able to resolve the issue.

To Reproduce

Steps to reproduce the behavior:

pnpx create-nx-workspace     # choose the apps preset and disabled cloud for now
pnpm add -D @nxext/ionic-angular
pnpm exec nx generate application     # here is where the errors should start to occur
pnpm exec nx run-many --target build     # if previous errors are fixed, more errors occur here
pnpm exec nx run-many --target test     # if previous errors are fixed, more errors occur here
pnpm exec nx run-many --target e2e     # with previous errors fixed, this ran fine for me
pnpm exec nx run-many --target lint     # with previous errors fixed, this ran fine for me

Expected behavior
src/generators/application/generator.js
An application should be generated which passes nx run-many --target build, nx run-many --target test, nx run-many --target e2e, and nx run-many --target lint.

Workaround

The workaround that works for me was to create a .pnpmfile.cjs file. That file can be found below.

Please note that the only packages I added to this file were packages where I received an error about not being able to find a module which was accompanied by a require-stack. For example, if it complained about not finding tslib and had a require-stack which started with the file <pnpm-private-hoisted-path>/@nrwl/storybook/src/generators/configuration/configuration.js, then I would cat that file and confirm that it does indeed rely on tslib in it's source, but doesn't list it in it's package.json. It can usually further be confirmed that some dependency in it's package.json probably includes this and if they weren't using pnpm, then it was likely hoisted to the root level which allowed them to require it in their source directly without specifying it in their package.json. That's how they ended up with a phantom dependency.

So my workaround is to "fake" that they had included tslib in their dependencies by adding such a dependency to the .pnpmfile.cjs with a comment about the offending source file. Then I can run pnpm install --fix-lockfile to patch those dependencies up. And I simply repeated this process until all the commands in the To Reproduce section ran successfully.

Here is that .pnpmfile.cjs (so far).

// https://pnpm.io/pnpmfile
module.exports = {
  hooks: {
    readPackage,
  },
};

const PHANTOM_DEPENDENCIES = Object.entries({
  '@nrwl/storybook': {
    // @nrwl/storybook/src/generators/configuration/configuration.js
    tslib: '*',
  },
  '@nrwl/workspace': {
    // @nrwl/workspace/src/utils/ast-utils.js
    '@angular-devkit/schematics': '*',
    '@angular-devkit/core': '*',
  },
  '@nxext/capacitor': {
    // @nxext/capacitor/src/generators/capacitor-project/generator.js
    tslib: '*',
    // @nxext/capacitor/src/generators/capacitor-project/lib/update-project-gitignore.js
    ignore: '*',
  },
  '@nxext/ionic-angular': {
    // @nxext/ionic-angular/src/generators/application/generator.js
    tslib: '*',
    // @nxext/ionic-angular/src/generators/application/lib/add-angular.js
    '@nrwl/linter': '*',
    // @nxext/ionic-angular/src/generators/application/lib/external-schematic.js
    '@angular-devkit/schematics': '*',
  },
  'jest-preset-angular': {
    // jest-preset-angular/build/config/ng-jest-config.js
    'jest-util': '*',
  },
  nx: {
    // nx/src/adapter/ngcli-adapter.js
    '@angular-devkit/core': '*',
    '@angular-devkit/schematics': '*',
    '@angular-devkit/architect': '*',
  },
});

// https://pnpm.io/pnpmfile#hooksreadpackagepkg-context-pkg--promisepkg
function readPackage(packageJson, context) {
  for (const [packageName, phantomDependency] of PHANTOM_DEPENDENCIES) {
    addDependenciesIfMissing(
      packageJson,
      context,
      packageName,
      phantomDependency
    );
  }
  return packageJson;
}

const DEPENDENCIES_PATCHED = Symbol('dependencies patched');

function addDependenciesIfMissing(
  packageJson,
  context,
  packageName,
  dependencies
) {
  // if we've already checked this packageJson, we to modify it again
  // this helps avoid erroneous messages about a dependency already being added
  // when it was actually added by us
  if (packageJson[DEPENDENCIES_PATCHED]) return;

  // if this isn't our package, skip it
  if (packageJson.name !== packageName) return;

  // we've checked this package, no need to check it again in the future
  packageJson[DEPENDENCIES_PATCHED] = true;

  // check each dependency
  for (const entry of Object.entries(dependencies)) {
    addDependencyIfMissing(packageJson, context, entry);
  }
}

function addDependencyIfMissing(packageJson, context, [dependency, version]) {
  // if the dependency exists already, don't overwrite it
  const currentVersion = packageJson.dependencies[dependency];
  if (currentVersion) {
    const message =
      `${packageJson.name} already has ${dependency}@${currentVersion}; ` +
      `ignoring attempt to patch with version ${version}; ` +
      `please update .pnpmfile.cjs as patching dependency is no longer required`;
    context.log(message);
    return;
  }

  // add the dependency
  packageJson.dependencies[dependency] = version;

  // and issue a log so we know what is happening
  const message =
    `patched ${packageJson.name} dependencies with ${dependency}@${version}; ` +
    `consider encouraging maintainers of ${packageJson.name} to update the dependencies`;
  context.log(message);
}

This workaround was originally recommended in the @nrwl/nx repository on an issue I raised. They resolved the issue by eventually adding dotenv to their dependencies. I have another issue out that is trying to address the same situation with fs-extra.

Additional Context

By default, pnpm does not publicly hoist dependencies. Any responses to "just publicly hoist your dependencies" will be ignored as not acceptable solutions because hoisting dependencies to the root is not recommended by pnpm. Publicly-hoisting dependencies creates "phantom dependencies"; read more. I did not try using the hoist-pattern option in .npmrc just yet. That might be a simpler solution than the .pnpmfile.cjs approach, but I have not been able to confirm that yet.

Here is some additional version / environment information that may or may not be relevant:

  • I'm on Ubuntu 20.04 at the moment
  • My colleagues on MacOS and Linux Manjaro are not able to reproduce this issue and as to why that is I still do not yet know
  • node --version = v16.16.0
  • pnpm --version = 7.11.0

Recommended Solution

@nxext/ionic-angular is not the only offending package. Other packages which have phantom dependencies will be addressed in separate issues which may reference this original issue. However, here are the ways in which @nxext/ionic-angular can help ensure that their packages work with pnpm when root-level hoisting is disabled (the default).

  • @nxext/capacitor
    • @nxext/capacitor/src/generators/capacitor-project/generator.js depends on tslib directly; please add tslib (at an appropriate version) to this library's dependencies (or peer dependencies)
    • @nxext/capacitor/src/generators/capacitor-project/generator.js depends on ignore directly; please add ignore (at an appropriate version) to this library's dependencies (or peer dependencies)
  • @nxext/ionic-angular
    • @nxext/ionic-angular/src/generators/application/generator.js depends on tslib directly; please add tslib (at an appropriate version) to this library's dependencies (or peer dependencies)
    • @nxext/ionic-angular/src/generators/application/lib/add-angular.js depends on @nrwl/linter directly; please add @nrwl/linter (at an appropriate version) to this library's dependencies (or peer dependencies)
    • @nxext/ionic-angular/src/generators/application/lib/external-schematic.js depends on @angular-devkit/schematics directly; please add @angular-devkit/schematics (at an appropriate version) to this library's dependencies (or peer dependencies)
  • the schematic @nxext/ionic-angular:application runs @angular-devkit/schematics, so the @nxext/ionic-angular:application schematic could improve by checking for and installing @angular-devkit/schematics when it checks for and installs other libraries that it depends on in the user's Nx monorepo
@troywweber7
Copy link
Author

Another phantom dependency to consider:

  • @nxext/ionic-angular/src/generators/page/lib/update-routing-file.js directly depends on:
    • @phenomnomnominal/tsquery

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant