denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
94.06k stars 5.23k forks source link

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

Open dsherret opened 1 year ago

dsherret commented 1 year ago

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.

grippy commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@dsherret I created this issue: https://github.com/denoland/deno/issues/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. 🤷‍♂️