tape-testing / tape

tap-producing test harness for node and browsers
MIT License
5.77k stars 307 forks source link

fix: fixed package path not exported: ./bin/tape #530

Closed Creplav closed 3 years ago

Creplav commented 3 years ago

I was having an error every time I would use a package that included tape regarding an export that would prevent me from continuing my pipelines.

The error was Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './bin/tape' is not defined by "exports" in .../node_modules/tape/package.json

This is a simple fix for that issue.

ljharb commented 3 years ago

Can you elaborate on this? "exports" is only for things that are required or imported - the tape binary should only be executed, never required.

redbar0n commented 3 years ago

The tape binary is required downstream by libraries building on tape: https://github.com/ericelliott/riteway/blob/e647631c3abb4b05e1edb2bd69dde4d282072eef/bin/riteway#L3

Creplav commented 3 years ago

The tape binary is required downstream by libraries building on tape: https://github.com/ericelliott/riteway/blob/e647631c3abb4b05e1edb2bd69dde4d282072eef/bin/riteway#L3

This is what I was referring to in the original post. When the test suite is run, it fails with the error above. Any time the package.json file is updated, this error comes back.

ljharb commented 3 years ago

Thanks, that's very surprising and not how I'd ever expect it to be used.

I think the proper thing for downstream tools to do is not to require an executable file, but to execute it, a la tape $@. If there's code they want to directly reuse, that could be moved into a separate file that's added to "exports". In other words, I think tools like riteway need to alter their approach.

ericelliott commented 3 years ago

@ljharb We use tape like a library and build on top of it in order to override the assertion API. I'm not sure how that could work if we just execute the bin.

const withTape = tapeFn => (unit = '', TestFunction = noop) => tapeFn(unit, withRiteway(TestFunction));

// The testing library: a thin wrapper around tape
const describe = Object.assign(withTape(tape), {
  only: withTape(tape.only),
  skip: tape.skip
});
ljharb commented 3 years ago

@ericelliott sure, but a binary executable is not a library and it's not reasonable to expect to use it like one.

You can execute the bin with --require, and patch tape itself before the tests run.

ericelliott commented 3 years ago

workflow

ericelliott commented 3 years ago

https://github.com/ericelliott/riteway/pull/293/commits/fe0b0a8efc65cc39d00154710b5a83711383e99c

ljharb commented 3 years ago

i mean sure, that's one way to do it, but i fail to see why you wouldn't just execSync out to tape with a --require for the patches - that way you don't have to manually maintain copypasta.

Either way, since this has multiple solutions, I'll close this PR. Executables are not part of (anyone's) requireable/importable API.

ericelliott commented 3 years ago

i mean sure, that's one way to do it, but i fail to see why you wouldn't just execSync out to tape with a --require for the patches - that way you don't have to manually maintain copypasta.

Mainly because I don't have time to figure it out. 😆