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

TypeError: this.npmInstall is not a function #1283

Closed
Lsnsh opened this issue Feb 28, 2021 · 6 comments
Closed

TypeError: this.npmInstall is not a function #1283

Lsnsh opened this issue Feb 28, 2021 · 6 comments
Labels
needs triage Awaiting triage

Comments

@Lsnsh
Copy link

Lsnsh commented Feb 28, 2021

Error log:

➜  created-by-my-cli git:(main) ✗ yoe run my-cli
TypeError: this.npmInstall is not a function
    at module.exports.writing (/Users/lsnsh/playground/my-cli/generators/app/index.js:25:10)
    at Object.<anonymous> (/Users/lsnsh/playground/my-cli/node_modules/yeoman-generator/lib/index.js:960:25)
    at /Users/lsnsh/playground/my-cli/node_modules/run-async/index.js:49:25
    at new Promise (<anonymous>)
    at /Users/lsnsh/playground/my-cli/node_modules/run-async/index.js:26:19
    at runLoop.add.once.once (/Users/lsnsh/playground/my-cli/node_modules/yeoman-generator/lib/index.js:961:11)
    at Immediate.<anonymous> (/Users/lsnsh/.nvm/versions/node/v12.16.2/lib/node_modules/yeoman-environment/node_modules/grouped-queue/lib/subqueue.js:48:34)
    at processImmediate (internal/timers.js:456:21)

generators/app/index.js:

var Generator = require('yeoman-generator');

module.exports = class extends Generator {
  // The name `constructor` is important here
  constructor(args, opts) {
    // Calling the super constructor is important so our generator is correctly set up
    super(args, opts);

    // Next, add your custom code
    this.option('babel'); // This method adds support for a `--babel` flag
  }

  writing() {
    const pkgJson = {
      devDependencies: {
        eslint: '^3.15.0'
      },
      dependencies: {
        react: '^16.2.0'
      }
    };

    // Extend or create package.json file in destination path
    this.fs.extendJSON(this.destinationPath('package.json'), pkgJson);
    this.npmInstall(['lodash'], { 'save-dev': true });
  }
};

Enviorment:

  • yo v3.1.1
  • yeoman-generator v5.0.1
  • yeoman-environment v3.0.1
@Lsnsh Lsnsh added the needs triage Awaiting triage label Feb 28, 2021
@mshima
Copy link
Member

mshima commented Feb 28, 2021

See https://github.com/yeoman/generator/releases/tag/v5.0.0.
I will keep open, so if the new approach is not enough, we can revert and include install mixin by default.

@esatterwhite
Copy link

I think think this is a bit less maintainable. the new method appears to require you to fix a version with the dependency. In most cases I don't want to specify a version and jsut let the most recent version be installed.

I'm also having a bit of trouble actually getting it to update package.json. In the debug logs, I can see something resolving packages from npm, but the package dependencies are never added

@esatterwhite
Copy link

What was the reasoning for deprecating this behavior?

@mshima
Copy link
Member

mshima commented Mar 8, 2021

What was the reasoning for deprecating this behavior?

@esatterwhite see #1067 (comment).
npmInstall should work for simple generators (easy to load the deprecated mixin too).
But it's not friendly when composing with multiples generators.

  • package manager conflicts (each generator executes it's preferred package manager).
  • multiples install executions when generating (each generator trigger it's own install).
  • not integrated with package.json changes (the install/end priority may have an outdated package.json in memory, since install was executed directly in the disk).

I think think this is a bit less maintainable. the new method appears to require you to fix a version with the dependency. In most cases I don't want to specify a version and jsut let the most recent version be installed.

Should be fixed, when the version is empty, it should add latest version to package.json.
I should address this problem soon, but feel free to open an issue for it.

I'm also having a bit of trouble actually getting it to update package.json. In the debug logs, I can see something resolving packages from npm, but the package dependencies are never added

If so, it's a bug, please open a new issue detailing the behavior.

@esatterwhite
Copy link

It doesn't look like the new methods have a way to specify a working directory and its always updating the package.json in the process cwd.

I maybe missing something too, is there away to modify package.json in a different directory?

@mshima
Copy link
Member

mshima commented Mar 10, 2021

It updates package.json relative to the generator destinationPath().

  1. Api could be improved by passing a storage instead of always using this.packageJson storage.
this.addDependencies({foo: ''}, this.createStorage('packages/my-project/package.json');
  1. Taking a mono-repo as examples.
    The correct approach is to have a generator for the mono-repo called workspaces.
    And another generator for the workspace (project).
    Each project generator should be composed by passing destinationRoot.
    Eg: from workspaces generator:
    composeWith('workspace', {destinationRoot: this.destinationPath('packages/my-project'});
    So package.json will be relative to 'packages/my-project'.
    Detail: composeWith forwards the current destinationRoot to it's child by default, so it's chained.

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

No branches or pull requests

3 participants