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

lerna run --no-bail throws error #149

Closed
5 tasks done
alex-taxiera opened this issue May 8, 2022 · 6 comments · Fixed by #155
Closed
5 tasks done

lerna run --no-bail throws error #149

alex-taxiera opened this issue May 8, 2022 · 6 comments · Fixed by #155
Labels
bug Something isn't working

Comments

@alex-taxiera
Copy link

Describe the bug

Migrating to npm workspaces + lerna-lite from lerna and was trying to run my same lint command with --no-bail flag.
The command seems to work completely but throws an error after the success report. Tried the same command without the --no-bail flag and it passed just fine.

Reproduction

run any command with --no-bail

Lerna config and logs

lerna.json

{
  "packages": [
    "packages/*"
  ],
  "stream": true,
  "version": "independent"
}

lerna-debug.log

0 silly argv {
0 silly argv   _: [ 'run' ],
0 silly argv   bail: false,
0 silly argv   lernaVersion: '1.1.1',
0 silly argv   '$0': '/Users/alex/Source/eris-boiler/node_modules/.bin/lerna',
0 silly argv   script: 'lint'
0 silly argv }
1 notice cli v1.1.1
2 verbose rootPath /Users/alex/Source/eris-boiler
3 info versioning independent
4 verbose project packages packages/*
5 info Executing command in 3 packages: "npm run lint"
6 verbose Topological npm run lint
7 success run Ran npm script 'lint' in 3 packages in 8.0s:
8 error TypeError: Cannot read properties of undefined (reading 'name')
8 error     at /Users/alex/Source/eris-boiler/node_modules/@lerna-lite/run/dist/run-command.js:119:66
8 error     at Array.forEach (<anonymous>)
8 error     at /Users/alex/Source/eris-boiler/node_modules/@lerna-lite/run/dist/run-command.js:113:25

Environment Info

| Executable        | Version |
| ----------------- | ------- |
| `lerna --version` | 1.1.1 |
| `npm --version`   | 8.2.0 |
| `node --version`  | 16.13.1 |
---

Used Package Manager

npm

Validations

@ghiscoding
Copy link
Member

The code is the same as the original Lerna except that I rewrote it in TypeScript and I never used that flag, so I can't provide much help on this,. I would suggest to look at Lerna's issues to see if you can find anything that might help there. Or troubleshoot and create a Pull Request if you ever come up with a fix. The option seems to be used at this line in the run command

https://github.com/ghiscoding/lerna-lite/blob/c6f3a3426b2c6c49efe69408f3094e3071e989d9/packages/run/src/run-command.ts#L105-L112

the lerna debug log file that you provided might be helpful though, it says that it can't read name, can you go and find out what that object it's trying to resolves to?

@ghiscoding
Copy link
Member

ghiscoding commented May 9, 2022

looking at the shipped code on unpkg, the line number don't match totally but it seems to say that the result.pkg is undefined and so result.pkg.name throws the error.

image

the only way to troubleshoot this would be for you to edit that JS file and add a console log of the result, the object structure might be different compare to what it expect. The success uses pkg.name instead of result.pkg.name so it might be what we have to change but still wonder what content result has

@alex-taxiera
Copy link
Author

I tried checking lerna for this exact code block but I could not find it. Modified the code with a log like you suggested, not sure exactly what result is supposed to be, but it looks like it's completely missing pkg in my use case.

modified code:

if (!this.bail) {
    results.forEach((result) => {
        console.log(result)
        if (result === null || result === void 0 ? void 0 : result.failed) {
            this.logger.error('', `- ${result.pkg.name}`);
        }
        else {
            this.logger.success('', ` - ${result.pkg.name}`);
        }
    });
}

result from the log:

{
  command: 'npm run lint',
  escapedCommand: 'npm run lint',
  exitCode: 0,
  stdout: '\n' +
    '> @hephaestus/utils@0.0.0 lint\n' +
    '> eslint src\n' +
    '\n' +
    '\n' +
    '/Users/alex/Source/eris-boiler/packages/utils/src/type.ts\n' +
    '   1:44  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any\n' +
    '  16:13  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any\n' +
    '\n' +
    '✖ 2 problems (0 errors, 2 warnings)\n',
  stderr: '',
  all: undefined,
  failed: false,
  timedOut: false,
  isCanceled: false,
  killed: false
}

@ghiscoding
Copy link
Member

ghiscoding commented May 9, 2022

Actually it's probably because I replicated this open PR from a Lerna contribution and that PR was probably not tested properly (it has no unit test for it). That is probably the cause. I still need to see the content of result to find the proper way to fix this. I would certainly like to have a PR if you find how to fix it. Thanks

I think it might be to simply change result.pkg.name to pkg.name (same as the success code) since result doesn't include the package itself. You could give that a try in your JS file. that probably won't work since that was a map from this.packagesWithScript. We could also revert that PR if it's causing more problem than it solves, it was probably a bad idea to copy contributions that didn't have proper unit tests 😆

@ghiscoding ghiscoding added the bug Something isn't working label May 9, 2022
@ghiscoding
Copy link
Member

ghiscoding commented May 10, 2022

@alex-taxiera
I found the issue with that PR from Lerna that I merged in Lerna-Lite, so that PR was only supposed to help the log output by showing the package name in the log when the run command is called without the --stream option (which you are using). Basically if you had called --no-bail without stream then you'd be ok, considering this if you modify Lerna-Lite JS file from dist folder with the following it should be working as expected (even with streaming enabled).

- if (!this.bail) {
+ if (!this.bail && !this.options.stream) {
        results.forEach((result) => {
          if (result?.failed) {
            this.logger.error('', `- ${result.pkg.name}`);
          } else {
            this.logger.success('', ` - ${result.pkg.name}`);
          }
        });
} else {
        this.logger.success('', this.packagesWithScript.map((pkg) => `- ${pkg.name}`).join('\n'));
}

It would be nice if you can confirm that it fixes the issue. From my end it seems to be all good.
Thanks

@ghiscoding
Copy link
Member

should be fixed in latest release 1.2.0

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

Successfully merging a pull request may close this issue.

2 participants