fix(taskr): use createRequire to perform relative requires and add missing optional peer dependency @taskr/esnext

[merceyz]

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 createRequire to perform the require relative to the base https://nodejs.org/api/module.html#module_module_createrequire_filename
• Add @taskr/esnext as a optional peer dependency

cc @lukeed @arcanis

[lukeed]

Thank you, but I think just defining peerDep is all that's needed. Installers will preserve access to the taskr/esnext

[merceyz]

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?

[lukeed]

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.

[merceyz]

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

[lukeed]

Can you please include or list steps for a simple reproduction?

[merceyz]

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

[merceyz]

@lukeed Anything else you'd like me to do to move this forward?

[lukeed]

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);
  }
}

[merceyz]

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 >=node@4.6[1] and the paths option wasn't added until node@8.9.0[2] would need a polyfill anyways.

1:

taskr/packages/taskr/package.json

Line 52 in 82d0bef

"node": ">= 4.6"

2: https://nodejs.org/api/modules.html#modules_require_resolve_request_options

[merceyz]

@lukeed Sorry to ping again but would be nice to get this merged