Support usage when started by 'ts-node' by using 'process.exe…

[stenzengel]

…cPath' instead of 'process.argv[0]'

As described in https://github.com/Mogztter/asciidoctor-kroki/issues/115, the creation of a Node.js child process fails, if "ts-node" is used instead of "node" when using "asciidoctor-kroki". This PR uses process.execPath instead of process.argv[0 which works for me in "asciidoctor-kroki" in both cases (see https://nodejs.org/api/process.html#process_process_execpath). I did not replace execSync with fork as described in the original issue, because test cases failed, but looked at the implementation of fork (https://github.com/nodejs/node/blob/ab895bd587ed47423a6baab0bd6934128aa8990d/lib/child_process.js#L140).

[ggrossetie]

That's really nice! Nice finding 👌 process.argv[0] was a bit of a hack so I'm glad to replace it with this property!

I wonder if it would be possible to run a test with ts-node? I guess we could add ts-node and typescript as dev dependencies and create a simple test.ts? What do you think?

[stenzengel]

I was afraid you would say that about tests... ;-) And you're right, of course. I'll try.

[ggrossetie]

I was afraid you would say that about tests... ;-)

Don't be afraid, your solution is really neat! I think it's important to add a test to make sure that we don't accidentally break it in the future.

And you're right, of course. I'll try.

Thanks 👍

[stenzengel]

@Mogztter I have pushed a first version of a test which is executed with "ts-node" which fails when using the original version with process.argv[0] and which passes when using my patched version with process.execPath. Could you please have a look at it?

I did not provide any configuration for TypeScript (i.e. there is no tsconfig.json and defaults are used). There is no linting ("eslint" with "typescript-eslint" could be used). It is just the style I got (i.e. semicolons at line ends, ...), when saving with my default VS Code settings. There are some as any type assertions which I don't like, etc.

In summary, there is a lot of room for improvement. Please feel free to modify or let me know if you want something to be changed.

[stenzengel]

@Mogztter There is an easier alternative to test, if XMLHttpRequest.js works when started with ts-node. It needs less new package dependencies than my last commit (no @type-packages), no additional code and does not introduce TypeScript files: Since JavaScript is valid TypeScript, we could simply run all the test cases (or in 2nd pass in addition) with ts-node node_modules/mocha/lib/cli/cli.js tests/test-*.js instead of mocha tests/test-*.js. (This stills fails without my process.execPath fix and passes with the fix).

Which one do you prefer?

The last approach (i.e. without the ts file) is simpler, but the initial approach (i.e. with the ts file) would be the first step of a future migration (e.g. for the test cases) to TypeScript.

[ggrossetie]

@stenzengel sorry I didn't have time to review it yet but your second proposal sounds good as we don't plan to migrate to TypeScript (but we want to make sure that this library is working fine with ts-node).

So running the exact same test file against node and ts-node seems to be the most logical solution.

[stenzengel]

@Mogztter Fine! I've simplified the code. I only run test-request.js with "ts-node", because it was the only one which failed before my fix . (I'm not quite sure that's what you were going for.) I kept the change for the optional parameters (https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#param-and-returns) in the JSDoc part, which solved a TS compile problem when I still used the TypeScript file. (I'm not sure which syntax you prefer.)

Joyeux Noël!

[ggrossetie]

Thanks @stenzengel and happy holidays to you :christmas_tree:

Fine! I've simplified the code. I only run test-request.js with "ts-node", because it was the only one which failed before my fix . (I'm not quite sure that's what you were going for.) I kept the change for the optional parameters (typescriptlang.org/docs/handbook/jsdoc-supported-types.html#param-and-returns) in the JSDoc part, which solved a TS compile problem when I still used the TypeScript file. (I'm not sure which syntax you prefer.)

Looks good :ok_hand:

[ggrossetie]

Now available as part of the 1.1.1 release, thanks again :tada:

[stenzengel]

It was a pleasure.