unable to use conf inside a webworker

[edenhermelin]

Hi there,

I'm using conf inside a webworker and experiencing some issues in making it work.
The webworker is initiated from Electron, but has nodeIntegrationInWorker and nodeIntegration at all places so the context there is of NodeJS, not browser ~ electron one.

(When I want to use config file from the main process of electron, Im using electron-store 😃 )

The error Im getting is the following:

After digging a little inside the error, it got me to these lines in conf's index.js:

as you can see here, the cache deletion is being targeted at the wrong path when using the __filename , probably on runtime this special variable is being translated into this not-so-nice electron path 😞

I also tried to use electron-store (although the webworker has a node context) but it gave this error:

Will be happy to get some advise about it, or if it's something that needs to be added to this awesome library, I'd be happy to contribute 😎

thanks!

[edenhermelin]

@sindresorhus I tried to run the source code from inside the webworker, and commenting out the line:
delete require.cache[__filename];
makes it possible to use the library without crash.

Is it possible to make this deletion optional / provide an alternative to __filename for this case?
a way to make it not resolve to this on runtime:

delete __webpack_require__.c[`${require("electron").remote.app.getAppPath()}/index.html`];

[sindresorhus]

Yeah, we should just remove that logic as it won't work with ESM anyway.

The problem is that it's inconvenient for every user to have to find the path to their package.json, load the JSON, parse it, and then get those properties out of it themselves. Maybe we could add a new option called importMeta at the same time. It could accept import.meta. We could then use import.meta.url to find the package.json. I'm open to other ideas.

[edenhermelin]

@sindresorhus it sounds like an interesting idea,
but Im not sure it will work for projects with webpack as of this annoying issue: webpack/webpack#6719
I tried to bundle with import.meta and it doesn't work well 😞

As another approach, we can add a new option resolvePackageJson which will default to true, but if it is sent as false -
the following part will be skipped:

delete require.cache[__filename];
const parentDir = path.dirname((_b = (_a = module.parent) === null || _a === void 0 ? void 0 : _a.filename) !== null && _b !== void 0 ? _b : '.');
.
.
.
 const getPackageData = onetime(() => {
      const packagePath = pkgUp.sync({ cwd: parentDir });
      // Can't use `require` because of Webpack being annoying:
      // https://github.com/webpack/webpack/issues/196
      const packageData = packagePath && JSON.parse(fs.readFileSync(packagePath, 'utf8'));
      return packageData !== null && packageData !== void 0 ? packageData : {};
    });

(also, I'd put the cache part inside a try-catch to avoid issues there)

This approach is not optimal (because as you mentioned - not everyone are comfortable with locating their package.json themselves), but it will allow to use conf for running inside workers.

For me it's a much bigger issue to migrate from conf (what I also really don't want to do as the library is super convenient), and it's easier to get the mandatory package.json properties (from what I saw in the source code, for the opts: projectVersion & projectName), or even parse the entire package.json if I need to.

I think that it's a very simple and quick solution for making it work for now, let me know what you think.
If you find this solution ok, I can issue a PR adding it quickly 😃

[sindresorhus]

As another approach, we can add a new option resolvePackageJson which will default to true, but if it is sent as false -
the following part will be skipped:

I don't think we need an option for that. You can just set the projectName and projectVersion options and getPackageData() will not be called. We could update the docs to recommend setting those options when using Webpack or when using a worker.

I think that it's a very simple and quick solution for making it work for now, let me know what you think.

I don't want to remove

conf/source/index.ts

Lines 29 to 32 in c92c032

	// Prevent caching of this module so module.parent is always accurate
	// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
	delete require.cache[__filename];
	const parentDir = path.dirname(module.parent?.filename ?? '.');

until there's a proper replacement as I described in my previous comment.

[edenhermelin]

@sindresorhus this sounds great, thanks.
The one thing that will be still mandatory to make it work when using a worker about :

conf/source/index.ts

Lines 29 to 32 in c92c032

	// Prevent caching of this module so module.parent is always accurate
	// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
	delete require.cache[__filename];
	const parentDir = path.dirname(module.parent?.filename ?? '.');

is to wrap it with try/catch, otherwise these lines make the app crash even when sending projectName and projectVersion.

Can the try/catch also be added there? (even if inside the catch nothing special happens?)

[sindresorhus]

Can the try/catch also be added there? (even if inside the catch nothing special happens?)

Sure

[edenhermelin]

@sindresorhus you rock ! 🎸 😎
let me know if I can help some how / PR / document / anything else

[edenhermelin]

@sindresorhus I opened a local branch with the addition of the try/catch around the delete require.cache part, but have no permissions to push it remotely. Can you please help ? 😃
the whole change is just :

let parentDir = '';
try {
// Prevent caching of this module so module.parent is always accurate.
// Note: This trick won't work with ESM or inside a webworker
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
	delete require.cache[__filename];
	parentDir = path.dirname(module.parent?.filename ?? '.');
}catch(e){}

*I noticed this opened PR, but Im not sure that it should work inside the webworker anyway