Misleading documentation

[gear4s]

Expected Behavior

The documentation for within() does not use cd() due to it touching process.cwd and not reverting in some way, but having no mention of this side-effect as part of within()

Actual Behavior

The documentation for within() uses cd() which changes the process.cwd of the entire script and introduces parity between process.cwd and $`cd`

Steps to Reproduce

1. Run this code for breaking example:

echo("Script start");
echo(`- ${await $`pwd`}`); // => cwd

await within(async () => {
  echo("First within");
  cd("/");
  echo(`- ${await $`pwd`}`); // => /
});

echo("Between within");
echo(`- ${await $`pwd`}`); // => cwd

await within(async () => {
  echo("Second within");
  echo(`- process CWD: ${process.cwd()}`); // => /
  echo(`- ${await $`pwd`}`); // => cwd
});

echo("Script end");
echo(`- ${await $`pwd`}`); // => cwd

Specifications

• zx version: 8.1.4
• Platform: macos
• Runtime: node

[antonmedv]

@antongolub please take a look, thanks.

[antongolub]

I see... cd() changes the global process.cwd, but internal $[processCwd] = process.cwd() assignment relates the local storage ctx only. So the root lvl $[processCwd] still keeps the initial default process.cwd() value. Previously we've used a sync hook to revert the cd side-effects for other within contexts, but since v8 is should be enabled manually via syncProcessCwd()

[gear4s]

@antongolub is there another API we could use to replace the context hook? seems like node upstream wants to remove it in its current capacity - maybe it should be reimplemented

[antongolub]

@gear4s, it looks as though it is going to be. Thanks for highlighting this. We'll keep that in mind.

@antonmedv, fyi.

https://nodejs.org/api/async_hooks.html#async-hooks

Stability: 1 - Experimental. Please migrate away from this API, if you can. We do not recommend using the createHook, AsyncHook, and executionAsyncResource APIs as they have usability issues, safety risks, and performance implications. Async context tracking use cases are better served by the stable AsyncLocalStorage API. If you have a use case for createHook, AsyncHook, or executionAsyncResource beyond the context tracking need solved by AsyncLocalStorage or diagnostics data currently provided by Diagnostics Chan

[antongolub]

@gear4s,

Could you tell us more about your practical example? Why do you need to switch the process dir in your case? Why not just $.pwd = '/' inside within callback?

[gear4s]

@gear4s,

Could you tell us more about your practical example?

I am building a tool to perform migrations of folders and files, and each migration has steps to execute. each step is executed in a within context. I am making pretty heavy use of fs.move within the within context, which only has context of process.cwd, which then results in Error: ENOENT: no such file or directory, open ....

effectively I am trying to do this

within(async () => {
  cd(`${basePath}/project/folder`);

  await $`sed -i '' 's|../path|../path2|' ./utils.js`;
  await $`sed -i '' 's|${__dirname}/../../|${__dirname}/../|' ./utils.js`;
  await $`sed -i '' 's|project/tests|tests|' ./folder/generator.sh`;
});

within(async () => {
  cd(basePath); // << failure happens here
  await fs.move(`./folder/folder`, `./otherFolder`);
  await fs.move(`./folder/folder2`, `./otherFolder2`);
});

[antonmedv]

So do we need to update some docs?

[antongolub]

We have a notice: https://google.github.io/zx/api#cd Should additional description be provided?

[antonmedv]

Let's add more documentation to https://google.github.io/zx/api#syncprocesscwd

More examples and explain what is the difference in terms of async contexts.

For example, without syncprocesscwd a different callback maybe out of sync.

[gear4s]

after doing some research into async hooks, do you guys think there could be another method of tracking cd in separate within contexts? using something like https://github.com/laverdet/isolated-vm or https://github.com/avoidwork/tiny-worker

[antonmedv]

Tiny-worker just spawns a process with different node script. This will not help here.

Isolates are cool. Bur also not applicable here.