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

deno install should warn when not ignoring auto-discovered configuration file #17855

Closed
dsherret opened this issue Feb 21, 2023 · 3 comments · Fixed by #27745
Closed

deno install should warn when not ignoring auto-discovered configuration file #17855

dsherret opened this issue Feb 21, 2023 · 3 comments · Fixed by #27745
Labels
feat new feature (which has been agreed to/accepted) suggestion suggestions for new features (yet to be agreed)

Comments

@dsherret
Copy link
Member

dsherret commented Feb 21, 2023

deno install is a global action, so it ignores the cwd's auto-discovered config file. It seems people are often confused by this, so we should update deno install to warn that the configuration file is ignored and needs to be explicitly.

For local files, we might want to consider auto-discovering based on the specified path.

@dsherret dsherret changed the title Deno install should warn when not ignoring auto-discovered configuration file deno install should warn when not ignoring auto-discovered configuration file Feb 21, 2023
@dsherret dsherret added the suggestion suggestions for new features (yet to be agreed) label Feb 21, 2023
@grippy
Copy link

grippy commented Apr 1, 2023

Not sure if this is related but I noticed deno install defaults to adding --no-config -- even if you don't pass this option. The deno install help reads like this should be explicit when calling this command.

For example:

deno install -A --root /usr/local --name my-program --force $PWD/cli/main.ts
✅ Successfully installed my-program
/usr/local/bin/my-program

Ends up with this script:

#!/bin/sh
# generated by deno install
exec deno run --allow-all --no-config 'file:///path/cli/main.ts' "$@"

I wouldn't have even noticed this if it wasn't for this issue:

While running the installed script:

error: Uncaught ReferenceError: __DENO_NODE_GLOBAL_THIS_1680362762__ is not defined
    at file:///Users/me/Library/Caches/deno/npm/registry.npmjs.org/ts-pattern/4.2.2/dist/index.module.js:1:18

If I remove the --no-config flag this issue goes away. Don't really understand the nuts and bolts of how the npm importing works and what not. But it seems like importing npm packages relies on the deno.lock and deno.jsonc file? Anyway, it might be worth telling the user this flag was added for them and that something might not work as expected. If this is a separate issue I'm happy to create it.

@dsherret
Copy link
Member Author

dsherret commented Apr 1, 2023

@grippy can you open a separate issue for that? I think it just happens to work in your case when removing the flag based on the cwd you're executing the command in (that's why the --no-config is there because it prevents auto-discovery of the cwd deno.json and package.json files).

@grippy
Copy link

grippy commented Apr 1, 2023

@dsherret I created this issue: #18551

I checked if the cwd is somehow making this work and it doesn't matter where I run the command from after removing --no-config. It works from any directory now. 🤷‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat new feature (which has been agreed to/accepted) suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants