node_shims icon indicating copy to clipboard operation
node_shims copied to clipboard

execPath still requires the use of deno

Open paoloricciuti opened this issue 2 years ago • 13 comments

https://github.com/denoland/node_shims/blob/094b9cfa50aac59e7b113816ddd8bbda40493194/packages/shim-deno/src/deno/stable/functions/execPath.ts#L5

I'm trying to build a cli with Deno and cross publish to npm using dnt but the cli works only if deno is installed and the above LOC should be the culprit. It tryes to find deno and throws if there's no deno installed.

Can i do something about this?

paoloricciuti avatar Jul 07 '23 16:07 paoloricciuti

Would you be able to show more details about your problem?

If the code is using Deno.execPath() to get the location of Deno, then this is correct for getting the location of Deno. If the code is using Deno.execPath() to not get the location of Deno, then probably that calling code should be changed.

dsherret avatar Jul 11 '23 12:07 dsherret

Would you be able to show more details about your problem?

If the code is using Deno.execPath() to get the location of Deno, then this is correct for getting the location of Deno. If the code is using Deno.execPath() to not get the location of Deno, then probably that calling code should be changed.

Sure...i'm writing this cli. It's quite simple and its using this https://deno.land/x/[email protected] to parse Deno.args and call the proper command.

https://github.com/SvelteLab/cli/tree/main/src

For the moment there's just a single command but actually even just trying to run npx sveltelab (that should only show the default message) results in an error when the user doesn't have deno installed.

➜  ~ npx sveltelab save https://www.sveltelab.dev/4n06i8am85quhu0 test
Need to install the following packages:
  [email protected]
Ok to proceed? (y) 
/Users/stanislav.khromov/.npm/_npx/aa144ceddd2d411a/node_modules/which/which.js:121
  throw getNotFoundError(cmd)
  ^

Error: not found: deno
    at getNotFoundError (/Users/stanislav.khromov/.npm/_npx/aa144ceddd2d411a/node_modules/which/which.js:10:17)
    at Function.whichSync [as sync] (/Users/stanislav.khromov/.npm/_npx/aa144ceddd2d411a/node_modules/which/which.js:121:9)
    at Object.execPath (/Users/stanislav.khromov/.npm/_npx/aa144ceddd2d411a/node_modules/@deno/shim-deno/dist/deno/stable/functions/execPath.js:9:40)
    at Object.<anonymous> (/Users/stanislav.khromov/.npm/_npx/aa144ceddd2d411a/node_modules/sveltelab/script/deps_2/deno.land/x/[email protected]/deps.js:64:28)
    at Module._compile (node:internal/modules/cjs/loader:1256:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
    at Module.load (node:internal/modules/cjs/loader:1119:32)
    at Module._load (node:internal/modules/cjs/loader:960:12)
    at Module.require (node:internal/modules/cjs/loader:1143:19)
    at require (node:internal/modules/cjs/helpers:110:18) {
  code: 'ENOENT'
}

Node.js v18.16.1

This is my build file that uses dnt

https://github.com/SvelteLab/cli/blob/main/tasks/build-npm.ts

paoloricciuti avatar Jul 11 '23 13:07 paoloricciuti

Ok apparently it's used here

https://deno.land/x/[email protected]/deps.ts?source

but given that this is shimming Deno...should this actually return the execPath of node?

paoloricciuti avatar Jul 11 '23 13:07 paoloricciuti

It looks like commander is asking for the location of Deno here:

https://github.com/acathur/cmd/blob/07339f4c7f8971c0a86e9371bf686b1549984fbd/deps.ts#L37

So I don't think this is an issue in these shims. It looks like cmd is specifically geared towards Deno and in this case it would need to be updated to handle possibly executing in Node.

What I'd recommend doing is upgrading dnt to the latest 0.37.0 version to get npm specifier support:

https://github.com/SvelteLab/cli/blob/6c9074a8912cfa27c51041f028c1c5382a9acec1/tasks/build-npm.ts#L1

Then change this line:

https://github.com/SvelteLab/cli/blob/6c9074a8912cfa27c51041f028c1c5382a9acec1/src/deps/commander.ts#L1

To something like:

import { Command } from 'npm:commander@^11.0';

That seems to make it work for me.

dsherret avatar Jul 11 '23 13:07 dsherret

It looks like commander is asking for the location of Deno here:

https://github.com/acathur/cmd/blob/07339f4c7f8971c0a86e9371bf686b1549984fbd/deps.ts#L37

So I don't think this is an issue in these shims. It looks like cmd is specifically geared towards Deno and in this case it would need to be updated to handle possibly executing in Node.

What I'd recommend doing is upgrading dnt to the latest 0.37.0 version to get npm specifier support:

https://github.com/SvelteLab/cli/blob/6c9074a8912cfa27c51041f028c1c5382a9acec1/tasks/build-npm.ts#L1

Then change this line:

https://github.com/SvelteLab/cli/blob/6c9074a8912cfa27c51041f028c1c5382a9acec1/src/deps/commander.ts#L1

To something like:

import { Command } from 'npm:commander@^11.0';

I don't know if you looked at my previous comment but what i'm asking is: if the goal of dnt (and more specifically node_shims) is to provide a way to cross publish deno and npm shouldnt this shim change the execPath from deno to node?

paoloricciuti avatar Jul 11 '23 13:07 paoloricciuti

Actually, you could just add the following when calling build to your build-npm.ts:

mappings: {
		'https://deno.land/x/[email protected]/mod.ts': {
			name: 'commander',
			version: '^11.0'
		}
	},

dsherret avatar Jul 11 '23 13:07 dsherret

I think i did that in the beginning but if i'm not mistaken the two packages have slightly different apis...i'll try tho

paoloricciuti avatar Jul 11 '23 13:07 paoloricciuti

I don't know if you looked at my previous comment but what i'm asking is: if the goal of dnt (and more specifically node_shims) is to provide a way to cross publish deno and npm shouldnt this shim change the execPath from deno to node?

Usually people will use execPath with arguments to launch a Deno application. The arguments are usually Deno specific and have stuff like run --allow-read=. main.ts and translating that to Node is very challenging to figure out.

For example, what would it do in this scenerio?

export function runDeno(args: string[]) {
  const denoExec = Deno.execPath();
  // code here that calls deno with the provided arguments
}

It's better for library authors to use something like https://github.com/dsherret/which_runtime to figure out what runtime the code is being run in for specific scenarios, then add some runtime specific behaviour.

dsherret avatar Jul 11 '23 13:07 dsherret

I don't know if you looked at my previous comment but what i'm asking is: if the goal of dnt (and more specifically node_shims) is to provide a way to cross publish deno and npm shouldnt this shim change the execPath from deno to node?

Usually people will use execPath with arguments to launch a Deno application. The arguments are usually Deno specific and have stuff like run --allow-read=. main.ts and translating that to Node is very challenging to figure out.

For example, what would it do in this scenerio?

export function runDeno(args: string[]) {
  const denoExec = Deno.execPath();
  // code here that calls deno with the provided arguments
}

It's better for library authors to use something like https://github.com/dsherret/which_runtime to figure out what runtime the code is being run in for specific scenarios, then add some runtime specific behaviour.

Mmm i guess it's fair...at the end of the day this should mostly be a problem for libraries that are ported from node to deno and try to recreate process in deno. That said i still think a shim should provide the "correct" path for the shimmed version.

However i completely understand and trust you on this decision 😄

Feel free to close this issue if you don't think it's a good idea to change this.

paoloricciuti avatar Jul 11 '23 13:07 paoloricciuti

It looks like commander is asking for the location of Deno here:

https://github.com/acathur/cmd/blob/07339f4c7f8971c0a86e9371bf686b1549984fbd/deps.ts#L37

So I don't think this is an issue in these shims. It looks like cmd is specifically geared towards Deno and in this case it would need to be updated to handle possibly executing in Node.

What I'd recommend doing is upgrading dnt to the latest 0.37.0 version to get npm specifier support:

https://github.com/SvelteLab/cli/blob/6c9074a8912cfa27c51041f028c1c5382a9acec1/tasks/build-npm.ts#L1

Then change this line:

https://github.com/SvelteLab/cli/blob/6c9074a8912cfa27c51041f028c1c5382a9acec1/src/deps/commander.ts#L1

To something like:

import { Command } from 'npm:commander@^11.0';

That seems to make it work for me.

I'm trying to use this but if i try to build with dnt it throws the follwing error

[dnt] Transforming...
error: Uncaught (in promise) TypeError: Unsupported scheme "npm:" for module "npm:commander@^11.0". Supported schemes: ["data:","blob:","file:","http:","https:"].
    throw new TypeError(
          ^
    at getValidatedScheme (https://deno.land/x/[email protected]/file_fetcher.ts:58:11)
    at FileFetcher.fetch (https://deno.land/x/[email protected]/file_fetcher.ts:229:20)
    at FetchCacher.load (https://deno.land/x/[email protected]/cache.ts:106:30)
    at fetch_specifier (https://deno.land/x/[email protected]/lib/pkg/snippets/dnt-wasm-a15ef721fa5290c5/helpers.js:6:22)
    at __wbg_fetchspecifier_bcacee1e00bcee02 (https://deno.land/x/[email protected]/lib/pkg/dnt_wasm.generated.js:271:21)
    at <anonymous> (https://deno.land/x/[email protected]/lib/pkg/dnt_wasm_bg.wasm:1:2124302)
    at <anonymous> (https://deno.land/x/[email protected]/lib/pkg/dnt_wasm_bg.wasm:1:1791039)
    at <anonymous> (https://deno.land/x/[email protected]/lib/pkg/dnt_wasm_bg.wasm:1:2715050)
    at <anonymous> (https://deno.land/x/[email protected]/lib/pkg/dnt_wasm_bg.wasm:1:1079734)
    at <anonymous> (https://deno.land/x/[email protected]/lib/pkg/dnt_wasm_bg.wasm:1:2632650)

is that expected behavior?

paoloricciuti avatar Jul 11 '23 14:07 paoloricciuti

Try this:

What I'd recommend doing is upgrading dnt to the latest 0.37.0 version to get npm specifier support:

https://github.com/SvelteLab/cli/blob/6c9074a8912cfa27c51041f028c1c5382a9acec1/tasks/build-npm.ts#L1

dsherret avatar Jul 11 '23 15:07 dsherret

Try this:

What I'd recommend doing is upgrading dnt to the latest 0.37.0 version to get npm specifier support: https://github.com/SvelteLab/cli/blob/6c9074a8912cfa27c51041f028c1c5382a9acec1/tasks/build-npm.ts#L1

Uh sorry didn't saw that. However just as an hjeads up version 0.35.0 and up errors out

error: Uncaught (in promise) TypeError: Deno.Command is not a constructor
    const process = new Deno.Command(cmd, {
                    ^
    at runCommand (https://deno.land/x/[email protected]/lib/utils.ts:46:21)
    at async build (https://deno.land/x/[email protected]/mod.ts:210:5)
    at async file:///C:/Users/Utente/Desktop/Vari/sveltelab_cli/tasks/build-npm.ts:12:1

However i was able to build with 0.34.0

paoloricciuti avatar Jul 11 '23 16:07 paoloricciuti

I think you're using an old version of Deno (deno upgrade will upgrade to the latest).

dsherret avatar Jul 11 '23 16:07 dsherret