Dexus / pem

Create private keys and certificates with node.js
Other
570 stars 129 forks source link

Jest tests display wrong line numbers when pem library (version 1.14.7 or 1.14.8) is added to project #389

Open josephmfaulkner opened 10 months ago

josephmfaulkner commented 10 months ago

It looks like the pem library is causing Jest tests to report the wrong line numbers for failed tests.

Initially, Jest displays the correct line that causes test to fail for a simple test. SCREENSHOT-correct-line-reported

When pem is added to the project (uncomment out the first line on testedComponent.js), jest now reports the same failed test on the wrong line number. SCREENSHOT-wrong-line-reported

This issue seems to have been present in the project since version 1.14.7 It doesn't reproduce for version 1.14.6 and below.

I've created a simple repository that reproduces this issue: https://github.com/josephmfaulkner/reproduce-jest-line-num-issue-when-using-pem

nwalters512 commented 8 months ago

I think you've independently discovered the same issue I spent all afternoon tracking down. Could you try something to see if this is indeed the same issue? In node_modules/pem/dist/index.js, change require('./sourcemap-register.js'); to /* require('./sourcemap-register.js'); */ on the first line and let me know if you still see the wrong line numbers?

nwalters512 commented 7 months ago

It seems like this can be resolved by importing from pem/lib/pem instead of just pem, e.g.:

import * as pem from 'pem/lib/pem';

@Dexus can you confirm if the lib directory is meant to be part of the public API? If so, I'll see about adding types for those to @types/pem so it's usable from TypeScript files, and possibly submitting a PR to this project documenting that in the README.

Can you share more about why the "main" field in package.json points to dist/index.js (which it looks like is bundled with ncc) as opposed to the unbundled files in lib? It seems like unbundled files are better all around, as they're possible to view/modify in node_modules and they avoid problems with sourcemaps like this issue describes.

Dexus commented 7 months ago

@nwalters512 pem/lib/pem is only a alternate to the bundled one. The different is that the pem bundeled does not push out all functions that the lib makes public. When I remember correctly.

nwalters512 commented 7 months ago

@Dexus thanks for the reply. By "alternate", you mean that it's safe to rely on importing from lib/ directly, I hope? Now I'm curious about why the bundled version exists in the first place. From what I can tell, that was introduced in #311, but I don't see any discussion about the motivation.

danfuzz commented 6 months ago

Hi! Just came here to note that pem's installing of its own sourcemap handler is problematic beyond just messing with error reports. See the linked bug above.

Beyond that, I don't actually want pem overriding globals in my system when not under test. It seems like there's a decent workaround, but maybe the fact that it does this by default should be called out in the docs. As with @nwalters512 I ended up spending quite a bit of frustrating time trying to figure out what was going wrong in my project.