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

Allow shebang in any file #336

Open
1 task
mk-pmb opened this issue Sep 10, 2024 · 13 comments
Open
1 task

Allow shebang in any file #336

mk-pmb opened this issue Sep 10, 2024 · 13 comments

Comments

@mk-pmb
Copy link

mk-pmb commented Sep 10, 2024

Rule details

I'd like to configure the shebang rule to do all of its regular checks, but not complain "This file needs no hashbang."

What type of rule is this?

Suggests an alternate way of doing something

Example code

#!/usr/bin/env -S nodemjs
import main from 'main.mjs';

main(process.argv.slice(2));

Participation

  • I am willing to submit a pull request to implement this rule.

Additional comments

Also the message "This file needs no hashbang." should be a bit more helpful, rather than just patronizing. When I first got the error, my first thought was, "Of course it needs one, how else would my shell know how to run it?" So maybe let's change it to something like "This file contains a hashbang but is not listed as a bin entry in package.json."

@scagood
Copy link

scagood commented Sep 10, 2024

There is an option in the rule for exactly this, it's called additionalExecutables

@mk-pmb
Copy link
Author

mk-pmb commented Sep 10, 2024

Thanks! I assumed that empty array in the example is an array of glob patterns, so I expressed my intention "any" as '**' and that worked. Now instead I need a way to restrict the enforcement of executableMap to the files mentioned in the "bin" entry.
Edit: Or, even better, can I give a regexp of acceptable hashbang lines? (Edit 2: Nope. "Value {} should be string.")
Edit 3: Also This file needs shebang "#!/usr/bin/env node". should probably say "hashbang" instead of "shebang".
Edit 4: Turns out this is not a solution either. With '**', now all files are expected to have a shebang. I want to just allow it, not enforce needless shebangs.

@aladdin-add
Copy link

there are 2 options:

  1. additionalExecutables: Mark files as executable that are not referenced by the package.json#bin property, default is [].
  2. ignoreUnpublished : Allow for files that are not published to npm to be ignored by this rule, default is false.

@mk-pmb
Copy link
Author

mk-pmb commented Sep 18, 2024

Neither of those work for me. I have scripts that are meant to be invoked by my node scripts, but would not usually be desirable to invoke from an interactive shell.

@aladdin-add
Copy link

I have scripts that are meant to be invoked by my node scripts, but would not usually be desirable to invoke from an interactive shell.

it seems an uncommon case; can you share an example to show how it's invoked in the node scripts?

@mk-pmb

This comment was marked as off-topic.

@mk-pmb
Copy link
Author

mk-pmb commented Oct 10, 2024

I salvaged the failed example from above by porting a simplified version of the CGI test from my other project. The test script uses cool node.js features (represented by encodeURIComponent) to construct a request for the CGI which uses cool node.js features (represented by decodeURIComponent) to parse its input.

@aladdin-add
Copy link

I see, thanks! you can safely disable the rule if your package.json does not contain pkg.bin field.

@aladdin-add
Copy link

aladdin-add commented Oct 10, 2024

  1. the testing files don't have to be published: you can use pkg.files or .npmignore. https://docs.npmjs.com/cli/v10/configuring-npm/package-json/?v=true#files

  2. If these files not to be published, you can use ignoreUnpublished: true, in which case none of these files will be reported.

@mk-pmb
Copy link
Author

mk-pmb commented Oct 10, 2024

In the original project, the CGI is meant to be published because CGI is the preferred way to use the module.

@mk-pmb
Copy link
Author

mk-pmb commented Oct 10, 2024

you can safely disable the rule if your package.json does not contain pkg.bin field.

The intent of my rules package is to avoid per-project rule definitions, so disabling the rule in every affected package would run counter to that. Disableing the entire rule would also disable all of its beneficial aspects. Could be a stopgap, but I hope we can do better.

Edit: I see now that you also wrote pkg.files above, so I assume you actually meant the bin field of my package.json. In that case it's void, because some of the CGIs are actually dual use for CLI and do have entries in the bin field.

@aladdin-add
Copy link

the files field is to configure which files will be published, please see: https://docs.npmjs.com/cli/v10/configuring-npm/package-json?v=true#files

@mk-pmb
Copy link
Author

mk-pmb commented Oct 10, 2024

Yes, that is the field that I deduced you probably meant with pkg.files. I have the impression that we are on the same page about all facts now, and have a case where none of the supposed solutions above work.

If it helps for motivation, I could make a 1:1 example that includes the bin field entries, has various CGIs outside the test directory, and explains why it would be useful to have most of these CGIs published to npm. However, I assume everyone here has enough imagination that the current example will suffice.

In my eyes it's now about who wants to take the burden of adding and maintaining a rule for the requested feature.

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

No branches or pull requests

3 participants