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

Documentation not up-to-date for "Install actions" #1294

Open
schorfES opened this issue May 5, 2021 · 25 comments
Open

Documentation not up-to-date for "Install actions" #1294

schorfES opened this issue May 5, 2021 · 25 comments

Comments

@schorfES
Copy link

schorfES commented May 5, 2021

Since version 5.0.0 the Install action is deprecated and is not included by default to the generator instance. This is currently not documented/reflected in here:

@schorfES schorfES added the needs triage Awaiting triage label May 5, 2021
@schorfES schorfES changed the title Documentation not up-to-date Documentation not up-to-date for "Install actions" May 5, 2021
@mshima mshima added help needed not stale and removed needs triage Awaiting triage labels May 5, 2021
@GregorBiswanger
Copy link

How is that best to be solved now? I urgently need an npm install solution. Tonight I have a live stream and wanted to show my viewers Yeoman with TypeScript.

@GregorBiswanger
Copy link

Okay, I saw that it then installed automatically. It reports that no standard package manager has been stored and that it uses npm. How can you fix a package manager like yarn here?

In principle, can the install method also be viewed as deprecated?

The TypeScript Type Definitions would also have to be adapted. Here npmInstall methods etc. are also offered.

@fernandopasik
Copy link

fernandopasik commented May 14, 2021

According to the 5.0.0 changelog there should be a way to access:

this.addDependencies({lit: '2.0.0'})
this.addDevDependencies({mocha: '8.4.0'})

@mshima
Copy link
Member

mshima commented May 15, 2021

Background about this change: #1283.

Force yarn using cli:
yo --node-package-manager yarn

Force yarn inside a generator:
this.env.options.nodePackageManager = yarn;

@GregorBiswanger
Copy link

We have created a generator that is supposed to create a new project directory with package.json. Only npm install will then not be executed automatically. Only if the package.json is created in the current directory where the generator is being executed. We tried this.destinationRoot(this.answers.kataname);

Although it shows during the debug that it has found the package.json, no installation is started.

Please urgently need a solution.

This is the generator:
https://github.com/GregorBiswanger/generator-create-kata

@mshima
Copy link
Member

mshima commented May 15, 2021

In that case you should set the environment cwd this.env.cwd = this.answers.kataname; too.
Or execute yarn using a custom install task:

generator/lib/index.js

Lines 127 to 128 in 84551ee

* @property {boolean|Function} customInstallTask - Provides a custom install task. Environment >= 3.2.0
* Environment built-in task will not be executed

Something like:

constructor(args, opts)
  super(args, opts, {customInstallTask: preferredPm => this.spawnCommandSync(`${preferredPm} install`)});
}

Edit:
Updated to use preferredPm

@GregorBiswanger
Copy link

We use npm :)

Thanks for your help!

@fernandopasik
Copy link

The TypeScript Type Definitions would also have to be adapted. Here npmInstall methods etc. are also offered.

@GregorBiswanger the definitions were updated a few hours ago
DefinitelyTyped/DefinitelyTyped#52666

@GregorBiswanger
Copy link

GregorBiswanger commented May 17, 2021

@mshima Unfortunately the cwd assignment does not work. He then runs npm install from global user profile c:\user\name. Although CWD gets the correct path:

    async prompting() {
        this.answers = await this.prompt(...

        const destinationRoot = this.destinationRoot(_.kebabCase(this.answers.kataname));
        this.env.cwd = destinationRoot;
    }

@gbrlmngr
Copy link

Any update on this?

I can't, for the life of me, find a way to configure installDependencies.
this.installDependencies always errors out with is not a function.

And also, I just cannot find a way to disable the unchanged package.json message (No change to package.json was detected. No package manager install will be executed.).

@gbrlmngr
Copy link

Any update on this?

I can't, for the life of me, find a way to configure installDependencies.
this.installDependencies always errors out with is not a function.

And also, I just cannot find a way to disable the unchanged package.json message (No change to package.json was detected. No package manager install will be executed.).

Downgrading yeoman-generator to version 4 seem to have fixed the issue.

@phatpham9
Copy link

@gbrlmngr just do like @mshima suggested, adding this.env.options.nodePackageManager = 'yarn'; to the generator solves the problem

@gbrlmngr
Copy link

gbrlmngr commented Jun 7, 2021

@gbrlmngr just do like @mshima suggested, adding this.env.options.nodePackageManager = 'yarn'; to the generator solves the problem

@phatpham9: Are you suggesting to specifically use yarn or to set nodePackageManager to some valid value (in my case, npm)?

@phatpham9
Copy link

phatpham9 commented Jun 7, 2021

@gbrlmngr just do like @mshima suggested, adding this.env.options.nodePackageManager = 'yarn'; to the generator solves the problem

@phatpham9: Are you suggesting to specifically use yarn or to set nodePackageManager to some valid value (in my case, npm)?

yeah if you want to force it to use npm, just replace the value with 'npm'

updated: don't forget to remove the install() {...} and add this.env.options.nodePackageManager = 'npm'; somewhere in your generator and it just works

@alexandra-c
Copy link

@gbrlmngr just do like @mshima suggested, adding this.env.options.nodePackageManager = 'yarn'; to the generator solves the problem

@phatpham9: Are you suggesting to specifically use yarn or to set nodePackageManager to some valid value (in my case, npm)?

yeah if you want to force it to use npm, just replace the value with 'npm'

updated: don't forget to remove the install() {...} and add this.env.options.nodePackageManager = 'npm'; somewhere in your generator and it just works

Hello,

I'm having the same issue like everybody here...
I tried your suggestion but when i first run my generator, the message No change to package.json was detected. No package manager install will be executed. still remains.
I need the installation to be executed when you first run the generator...if not this is pointless and the decision to remove the "install actions" is more a setback than an improvement.

Is there a way to trigger the installation of the packages manually?

@alexandra-c
Copy link

Any update on this?
I can't, for the life of me, find a way to configure installDependencies.
this.installDependencies always errors out with is not a function.
And also, I just cannot find a way to disable the unchanged package.json message (No change to package.json was detected. No package manager install will be executed.).

Downgrading yeoman-generator to version 4 seem to have fixed the issue.

Did this to get the old install behaviour back:

const Generator = require('yeoman-generator')
require('lodash').extend(Generator.prototype, require('yeoman-generator/lib/actions/install'))

Hopefully it will also be available in future releases until a more complete version of package.json manipulation will be released.

@gbrlmngr
Copy link

@alexandra-c downgrading to the previous major version worked for me, with everything working as expected.

@mshima
Copy link
Member

mshima commented Jul 5, 2021

An additional information.
package.json detection and install is done at conflicts priority, before install and end.
So addDependencies must be called before that.
I would need a minimal reproduction step.

@bgdopus
Copy link

bgdopus commented Jul 7, 2021

import { exec } from 'child_process';
...
const execAsync = util.promisify(exec);
....
public async install(){
try {
await execAsync(cd ${projectPath} && npm i);
} catch (error) {
throw Messages.INSTALLATION_FAILED_MESSAGE;
}
}

@skippednote
Copy link

Now that we are missing the cwd option yeoman is adding dependencies in the wrong folder. I tried changing directories using process.chdir but that doesn’t help either.
Here is the reference https://github.com/axelerant/kashmir/pull/125/files#diff-fb3c51194730e282793310751066bf26defbbee94fc60924bbde9d954f646e6fL116

@jwalton
Copy link

jwalton commented Aug 10, 2021

I would need a minimal reproduction step.

@mshima This is not quite minimal, but this branch of my generator has the problem: https://github.com/jwalton/node-jwalton-generator-typescript/tree/upgrade-yeoman

In my case, if you try to create a module with the same name as the current folder, I write it to the current folder, and it does the npm install as expected. If you try to create a module with a different name, I create a subfolder and then this.destinationRoot() to update the root folder:

    default() {
        if (path.basename(this.destinationPath()) !== this.props.githubProjectName) {
            this.log(`Creating folder ${this.props.githubProjectName}.`);
            mkdirp(this.props.githubProjectName);
            this.destinationRoot(this.destinationPath(this.props.githubProjectName));
        } else {
            this.log(`Not creating folder ${this.props.githubProjectName}.`);
        }
    }

And in this case I get the No change to package.json was detected. Seems like the check for package.json should be getting package.json from this.destinationRoot() instead of from this.cwd?

@spence-s
Copy link

spence-s commented Aug 24, 2021

I agree with @alexandra-c - this seems like an arbitrary set back to the api. But even worse - is the only way to figure out how things are working anymore is to read the actual code... Even the JSDoc is wrong/outdated.

@mshima - while your work is appreciated, and I know documentation is pain...this is a heavily depended upon package and changes affect a lot of people.

Do you think you could at least possibly find the time to update the JSDoc with all the code and API changes so that users aren't completely left hanging out to dry?

EDIT: Upon further looking and digging, it is documented in the changelog- for anyone else wondering.

@mneil
Copy link

mneil commented Jan 7, 2022

I ran into this today while trying to install packages where I need to pass options. It's unclear, with the automatic install, how to set options. Specifically, in my case, I want to omit some peer dependencies using npm 7 and need to pass a flag at install to --omit the specific dependencies I do not want.

To give an example of what I mean.

Package A:

{
  "peerDependencies": {
    "b": "^1.0.0",
    "c": "^1.0.0"
  }
}

Packages B and C are separate tools that invoke package A but package A never actually uses them. The user can choose to either bring in package B or C to execute package A - but they do not need both. With npm 7 dependency resolution changes so that peerDependencies that are not satisfied are brought in by default. This wouldn't be an issue except package B and C are very large and due to size limitations we can only fit B or C but not both.

Automatic install is fine. But it would be nice to have a way documented to set extra options during the npm install for this case and any others that need install flags / args.

@mshima
Copy link
Member

mshima commented Jan 8, 2022

But it would be nice to have a way documented to set extra options during the npm install for this case and any others that need install flags / args.

Npm is executed once and you can customize it.
See #1294 (comment)

@noahw3
Copy link

noahw3 commented Jan 22, 2022

Stitching bits and pieces of this thread together I was able to create a workaround:

  1. Set the customInstallTask flag when constructing the generator super(args, opts, { customInstallTask: true });
  2. Change the directory before installing dependencies
async install() {
  this.log.info('Installing yarn packages');
  await exec(`yarn install --cwd ${this.destinationPath()}`);
}

Overall though I agree with the sentiment that this is a behavioral regression. Being able to manage packages programmatically through yeoman is a potentially nice feature, but we shouldn't be forced to do it. If I just want to list all of the dependencies in the template file it shouldn't be hard to just install them once.

tbuschto added a commit to eclipsesource/generator-tabris-js that referenced this issue Jun 2, 2022
The current Yeoman generator version 5.6.x, while ours was. 3.1.x.
However, version 5 introduced breaking changes regarding npm package
management. Attempts to migrate to this new API failed as it is very
different, poorly documented and possibly broken. See
yeoman/generator#1294

Therefore migrate to latest 4.x only. It works out of the box.
tbuschto added a commit to eclipsesource/generator-tabris-js that referenced this issue Jun 3, 2022
The current Yeoman generator version is 5.6.x, while ours was 3.1.x.
However, version 5 introduced breaking changes regarding npm package
management. Attempts to migrate to this new API failed as it is very
different, poorly documented and possibly broken. See
yeoman/generator#1294

Therefore migrate to latest 4.x only. It works out of the box.
duncdrum added a commit to eXist-db/generator-exist that referenced this issue Jul 5, 2022
see yeoman/generator#1294

yo@5 is still not handling this well
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests