lukeed
fix(taskr): use createRequire to perform relative requires and add missing optional peer dependency @taskr/esnext
What's the problem this PR addresses?
taskr makes assumptions about the node_modules layout and tries to require files directly from it instead of using createRequire, taskr also has an optional peer dependency on @taskr/esnext that it doesn't declare making it rely heavily on hoisting to place it in an accesible location.
Read https://yarnpkg.com/advanced/rulebook for more in depth information
How did you fix it?
- Use
createRequireto perform the require relative to thebasehttps://nodejs.org/api/module.html#module_module_createrequire_filename - Add
@taskr/esnextas a optional peer dependency
cc @lukeed @arcanis
I think just defining peerDep is all that's needed. Installers will preserve access to the taskr/esnext
Do you mean just making @taskr/esnext a normal peerDependency, not optional?
No, still optional. You already have it listed as a peerDependency. That's enough to have taskr and @taskr/esnext linked if/when @taskr/esnext is installed.
Do you want me to remove the createRequire part? Can't really do that as it's needed for all the other plugins it attempts to access, not @taskr/esnext
Sure, here is a sh file that will reproduce the issue
mkdir taskr-repro
cd taskr-repro
printf "exports.lint = function * (task) {
yield task.source('src/**/*.js').prettier({
semi: false,
useTabs: true,
trailingComma: 'es5'
}).target('dist/js');
}" > taskfile.js
mkdir src
printf "console.log('foo'); // bar" > src/index.js
yarn init -y2p
yarn config set pnpFallbackMode none
yarn add taskr @taskr/prettier
PNP_DEBUG_LEVEL=1 yarn taskr lint
The issue is that taskr tries to require @taskr/prettier from itself, and then directly from /taskr-repro/node_modules/@taskr/prettier both are invalid because taskr can be hoisted higher up than @taskr/prettier and /taskr-repro/node_modules/@taskr/prettier isn't guaranteed to exist.
The solution is to perform the require relative to the users project which declares the plugins.
https://yarnpkg.com/advanced/rulebook#modules-shouldnt-hardcode-node_modules-paths-to-access-other-modules
Hey, sorry for the delay on this! Never got around to setting up yarn@2 (don't want to alter my environment)
Should't something like this work too? I have something like this in other places:
function req(name, base) {
try {
return require( require.resolve(name, { paths: [base] }) );
} catch (e) {
$.alert(e.message);
}
}
Hey, sorry for the delay on this!
No worries :)
Never got around to setting up yarn@2 (don't want to alter my environment)
Yarn@2 is installed on a per project basis so you would only alter a single project, the repro I provided does just that in the init command.
Should't something like this work too? I have something like this in other places:
Most likely yes, it's only in some rare cases where it doesn't quite work. However, since taskr is set to target >[email protected][1] and the paths option wasn't added until [email protected][2] would need a polyfill anyways.
1: https://github.com/lukeed/taskr/blob/82d0bef537c598d32ef425af7eef7cee3cf59a5f/packages/taskr/package.json#L52 2: https://nodejs.org/api/modules.html#modules_require_resolve_request_options