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

Dependency resolution is broken with 'yarn --pnp' (Yarn Plug 'n' Play) #12465

Closed
samsieber opened this issue Oct 3, 2018 · 13 comments
Closed
Labels
area: angular/cli feature Issue that requests a new feature freq1: low Only reported by a handful of users who observe it rarely severity2: inconvenient
Milestone

Comments

@samsieber
Copy link

Bug Report or Feature Request (mark with an x)

- [x] bug report -> please search issues before submitting
- [ ] feature request

Command (mark with an x)

- [ ] new
- [x] build
- [ ] serve
- [ ] test
- [ ] e2e
- [ ] generate
- [ ] add
- [ ] update
- [ ] lint
- [ ] xi18n
- [ ] run
- [ ] config
- [ ] help
- [ ] version
- [ ] doc

Versions

ng: 6.0.0
yarn: 1.12.0 (RC)
node: v8.11.3
os: OSX 10.13.6

Repro steps

  1. Install yarn RC 1.12.0
  2. clone https://github.com/samsieber/ng-demo
  3. go in the cloned repo
  4. run yarn --pnp (switches it to pnp mode)
  5. run yarn build

The log given by the failure

Could not find module "@angular-devkit/build-angular" from "/Users/ssieber/dev/ng-yarn-pnp".
Error: Could not find module "@angular-devkit/build-angular" from "/Users/ssieber/dev/ng-yarn-pnp".
    at Object.resolve (/Users/ssieber/Library/Caches/Yarn/v3/npm-@angular-devkit-core-0.6.8-3b09d97bd2588f0091df11921f7ed772431806aa/node_modules/@angular-devkit/core/node/resolve.js:141:11)
    at Observable.rxjs_1.Observable [as _subscribe] (/Users/ssieber/Library/Caches/Yarn/v3/npm-@angular-devkit-architect-0.6.8-977acc605aba45d21b95ca704cc99492e14299dd/node_modules/@angular-devkit/architect/src/architect.js:132:40)
    at Observable._trySubscribe (/Users/ssieber/Library/Caches/Yarn/v3/npm-rxjs-6.3.3-3c6a7fa420e844a81390fb1158a9ec614f4bad55/node_modules/rxjs/internal/Observable.js:44:25)
    at Observable.subscribe (/Users/ssieber/Library/Caches/Yarn/v3/npm-rxjs-6.3.3-3c6a7fa420e844a81390fb1158a9ec614f4bad55/node_modules/rxjs/internal/Observable.js:30:22)
    at DoOperator.call (/Users/ssieber/Library/Caches/Yarn/v3/npm-rxjs-6.3.3-3c6a7fa420e844a81390fb1158a9ec614f4bad55/node_modules/rxjs/internal/operators/tap.js:32:23)
    at Observable.subscribe (/Users/ssieber/Library/Caches/Yarn/v3/npm-rxjs-6.3.3-3c6a7fa420e844a81390fb1158a9ec614f4bad55/node_modules/rxjs/internal/Observable.js:25:22)
    at /Users/ssieber/Library/Caches/Yarn/v3/npm-rxjs-6.3.3-3c6a7fa420e844a81390fb1158a9ec614f4bad55/node_modules/rxjs/internal/util/subscribeTo.js:22:31
    at Object.subscribeToResult (/Users/ssieber/Library/Caches/Yarn/v3/npm-rxjs-6.3.3-3c6a7fa420e844a81390fb1158a9ec614f4bad55/node_modules/rxjs/internal/util/subscribeToResult.js:10:45)
    at MergeMapSubscriber._innerSub (/Users/ssieber/Library/Caches/Yarn/v3/npm-rxjs-6.3.3-3c6a7fa420e844a81390fb1158a9ec614f4bad55/node_modules/rxjs/internal/operators/mergeMap.js:82:29)
    at MergeMapSubscriber._tryNext (/Users/ssieber/Library/Caches/Yarn/v3/npm-rxjs-6.3.3-3c6a7fa420e844a81390fb1158a9ec614f4bad55/node_modules/rxjs/internal/operators/mergeMap.js:76:14)

Desired functionality

I'd like it work ;) - Specifically when I'm not using yarn pnp, I get this:

yarn run v1.12.0
warning ../package.json: No license field
$ ng build
                                                                                          
Date: 2018-10-03T18:44:42.889Z
Hash: df3a0ea990e4eeeb9455
Time: 8768ms
chunk {main} main.js, main.js.map (main) 25.6 kB [initial] [rendered]
chunk {polyfills} polyfills.js, polyfills.js.map (polyfills) 227 kB [initial] [rendered]
chunk {runtime} runtime.js, runtime.js.map (runtime) 5.22 kB [entry] [rendered]
chunk {styles} styles.js, styles.js.map (styles) 15.6 kB [initial] [rendered]
chunk {vendor} vendor.js, vendor.js.map (vendor) 3.54 MB [initial] [rendered]
✨  Done in 12.48s.

Mention any other details that might be useful

This is apparently caused the build tool implementing it's own dependency resolution. Yarn pnp provides an alternative dependency resolution that eschews node_modules. It's an experimental feature. I've filed an issue in their repo, but they directed me here, since you guys seem to be using custom resolution. See yarnpkg/yarn#6482

@samsieber samsieber changed the title Dependency resolution is broken with 'yarn --pnp' Dependency resolution is broken with 'yarn --pnp' (Yarn Plug 'n' Play) Oct 3, 2018
@arcanis
Copy link

arcanis commented Oct 3, 2018

As mentioned, Angular seems to do the resolution by itself, which causes issues when operating within exotic installation schemes where some assumptions don't necessarily hold. Fixes look quite easy though!

I've made a few quick fixes to see what's the minimal amount of changes needed to make it work, and listed them below. Let me know what you think, and if you want to setup a call to discuss all this in more details I'm all for it! Please ping me by email or Twitter or this thread 🙂

Missing dependencies

Plug'n'Play detected various unsafe behaviors that probably should be fixed whatever you choose to do:

@angular-devkit/build-angular

Cannot locate node_modules

Hmm, can I skip this one? That's kinda expected, I'd say 😛

Overall there's a bunch of node_modules assumptions there, so I guess it would make more sense for you to give it a look yourself - from what I gather it wouldn't be too much work to have something more generic. The cache folder can be set to .pnp/pkgdata/angular when you're in a PnP environment (instead of node_modules/something, which is risky), and the Webpack loaders can be easily (really!) loaded through the use of the pnp-webpack-plugin package. It's really just two extra lines in your configuration.

require-project-module.js

Basically just have to add a guard to directly use the PnP API when operating under a PnP environment (I can see the reticence to special case, but keep in mind this API is precisely meant to offer a common abstraction to how various projects could decide to layout the dependencies - in this sense it's more generic than resolve, which only supports node_modules).

function resolveProjectModule(root, moduleName) {
    if (process.versions.pnp) {
        return require('pnpapi').resolveRequest(moduleName, `${root}/`);
    } else {
        return resolve.sync(moduleName, { basedir: root });
    }
}

Note that this has to be done in @angular-devkit/build-angular as well (but I supposed the source code is mutualized between those two locations?).

@angular-devkit/core/node/resolve.js

Similarly to the previous note, a small edit is needed:

function resolve(x, options) {
    // ...
    const extensions = options.extensions || Object.keys(require.extensions);
    const basePath = options.basedir;
    if (process.versions.pnp) {
        const orig = x;
        const pnpapi = require('pnpapi');
        try {
            if (options.resolvePackageJson)
                x += `/package.json`;
            x = pnpapi.resolveRequest(x, `${basePath}/`, {extensions});
        } catch (error) {
            x = orig;
        }
    }
    // ...
}

@samsieber
Copy link
Author

samsieber commented Nov 27, 2018

Small update - yarn plug 'n play (pnp) is now usable on the latest stable yarn (1.12.3). It isn't widely advertised yet, so it's still low profile / probably still experimental.

@whyboris
Copy link

Confirming error persists with the latest versions:

  • yarn 1.13.0
  • angular 7.1.4
  • angular-cli 7.1.4
  • @angular-devkit/build-angular 0.11.4
Could not find module "@angular-devkit/build-angular" from "/Users/byakubchik/Desktop/yarn/publish".
Error: Could not find module "@angular-devkit/build-angular" from "/Users/byakubchik/Desktop/yarn/publish".

@samsieber
Copy link
Author

Another update - yarn pnp has official documentation now: https://yarnpkg.com/en/docs/pnp

Also, yarn is planning on making pnp the default mode in yarn 2.0: yarnpkg/yarn#6953

@cveld
Copy link

cveld commented Mar 24, 2019

Any update on if yarn pnp support will get added to Angular. And if so, when?

@arcanis
Copy link

arcanis commented Mar 24, 2019

Let me know if someone with Angular knowledge is interested to make a peer programming session to make the needed changes. I'd be happy to help 🙂

@mayfieldiv
Copy link

Update on Yarn 2: the first release candidate is available https://github.com/yarnpkg/berry/releases/tag/2019-08-16

The lead Yarn maintainer @arcanis seems eager to assist, so it'd be awesome if an Angular dev could find some time to work with him on this 😀

@laurencefass
Copy link

laurencefass commented Dec 10, 2019

this is still broken with yarn 1.19.1
raised related issue here: yarnpkg/yarn#7746

@arcanis
Copy link

arcanis commented Dec 10, 2019

@laurencefass Further Yarn development happen mostly on the v2 trunk 🙂

I'm happy to report that thanks to @Embraser01 and @larixer we're very close from a fully working Angular! I was actually about to merge our first Angular E2E test to track potential regressions.

@klemenoslaj
Copy link

It would be cool to have Angular CLI working with pnp. What is the progress here? Any help required?


In regards to pnp-webpack-plugin, that would only solve the builds of application projects, correct? Libraries built with ng-packagr would not be that easily adoptable.

I wish TypeScript would open their API to allow yarn team to actually hook in, but I get why they don't do it.

@larixer
Copy link

larixer commented Feb 25, 2020

While everyone can use whatever tool they feel appropriate, it is also important to note that Angular is supported by Yarn 2

  1. For PnP install strategy via @yarnpkg/pnpify tool.
  2. Yarn 2 also supports node_modules install strategy via nodeLinker: node-modules option.
    For details please check this pinned issue on Angular CLI:
    Yarn PnP Support Status #16980

@dgp1130 dgp1130 added triage #1 feature Issue that requests a new feature and removed type: bug/fix labels May 26, 2020
@dgp1130
Copy link
Collaborator

dgp1130 commented May 26, 2020

Duplicate of #16980.

We're working on Yarn 2.0 support and PnP in particular. Follow #16980 for updates on the effort.

@dgp1130 dgp1130 closed this as completed May 26, 2020
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: angular/cli feature Issue that requests a new feature freq1: low Only reported by a handful of users who observe it rarely severity2: inconvenient
Projects
None yet
Development

No branches or pull requests

10 participants