Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[feat] improve error on parsing files in package.json #8311

Open
niftylettuce opened this issue Aug 22, 2020 · 1 comment · May be fixed by #8370
Open

[feat] improve error on parsing files in package.json #8311

niftylettuce opened this issue Aug 22, 2020 · 1 comment · May be fixed by #8370

Comments

@niftylettuce
Copy link

Right now if you have a String instead of an Array, it will yield this error on yarn publish:

Trace: 
  TypeError: onlyFiles.map is not a function
      at /Users/jack/.yarn/lib/cli.js:42596:38
      at Generator.next (<anonymous>)
      at step (/Users/jack/.yarn/lib/cli.js:310:30)
      at /Users/jack/.yarn/lib/cli.js:321:13

I suggest that we improve this error message to check the type, and if it is not an Array, then alert to the user that the files property in package.json is a ${typeof prop} as opposed to erring on .map not being a function as it currently does.

Ref: #7815

@niftylettuce
Copy link
Author

Seems like we just need to change this:

// `files` field
if (onlyFiles) {
let lines = [
'*', // ignore all files except those that are explicitly included with a negation filter
];
lines = lines.concat(
onlyFiles.map((filename: string): string => `!${filename}`),
onlyFiles.map((filename: string): string => `!${path.join(filename, '**')}`),
);
const regexes = ignoreLinesToRegex(lines, './');
filters = filters.concat(regexes);
}
.

To something like:

  // `files` field
  if (onlyFiles) {
+    if (!(onlyFiles instanceof Array)) throw new MessageError('"files" property in package.json must be an Array');
    let lines = [
      '*', // ignore all files except those that are explicitly included with a negation filter
    ];
    lines = lines.concat(
      onlyFiles.map((filename: string): string => `!${filename}`),
      onlyFiles.map((filename: string): string => `!${path.join(filename, '**')}`),
    );
    const regexes = ignoreLinesToRegex(lines, './');
    filters = filters.concat(regexes);
  }

niftylettuce added a commit to ladjs/express-redirect-loop that referenced this issue Aug 22, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants