Skip to content

test: make tmpdir.js importable with destructuring/named export

Rodrigo Muino Tomonari requested to merge github/fork/GeoffreyBooth/fix-test-tmpdir into main

Very small PR to fix that test/common/tmpdir.js throws this error if you run its refresh() from an ESM test:

~/node/test/common/tmpdir.js:22
  rmSync(this.path);
              ^

TypeError: Cannot read properties of undefined (reading 'path')

I’m not sure why this was written to use this.path in the first place, since when I inspect it it always seems to be the same value as tmpPath defined just above; so I put that in instead. Seems like a simple fix and gets rid of this error. (I’m working on a separate PR that imports this file from an ESM test, which is why I needed this fixed.)

You can reproduce this by creating test/foo.mjs as follows:

import { refresh } from './common/tmpdir.js';
refresh();

And running it via ./node test/foo.mjs

A perhaps bigger question for @nodejs/modules and @nodejs/loaders is why the value of this within this function varies based on whether the file is imported by a CommonJS consumer as opposed to an ESM consumer. I don’t think this is something to be resolved in this PR, but someone who understands this stuff better then me should please comment explaining why this is expected behavior (or if it’s not, then we should open a bug issue).

Merge request reports