search

mikeal / ipjs

Universal JavaScript Build and Packaging
41 stars 4 forks source link

fix: work with jest #14

Closed achingbrain closed 3 years ago

achingbrain commented 3 years ago

Jest doesn't support package exports, nor does it support browser overrides out of the box (though it can be configured).

This means it parses the stubbed files introduced in #13 as javascript, so let's just require and export the file that the stub is stubbing.

This has the added bonus of also supporting old nodes that don't understand package exports.

Fixes achingbrain/uint8arrays#21

rvagg commented 3 years ago

yeesh, this is a nasty fix; I'm getting a file without an extension now for every named export and the index.js is screwing up type generation inside the dist/ directory which I have for a lot of packages so I'm having to add it to index.js.

I'd like to flag this for reversion as soon as we get movement on Jest. I don't think they can hold out for too much longer as it's getting increasingly painful for their users. There are workarounds (and my opinion on this issue is that it's up to users to deal with it and put pressure on Jest, not us to pander to this weird environment) so this isn't unworkable for them. But there's also zero movement on browser-resolve, it's complicated and Jordan doesn't seem to have time to implement changes there. I think Jest is going to end up switching resolving engines because of this. Once that happens, I'd like to yeet this change, it's really unpleasant.

rvagg commented 3 years ago

Example of what I'm having to do to fix the breakage that flows from this change: https://github.com/ipld/js-car/pull/39

For other packages where I'm actually using an index.js I'll have to rename that too.

rvagg commented 3 years ago

btw this is the solution I've been giving to people, which I implemented for js-dag-jose: https://github.com/ceramicnetwork/js-dag-jose/commit/51750b4266bc57ae56af05e0899acf38c519799b#diff-3f698d0dc0e17487612dbe228105aa820683a2eb38343929c1c45d9a8aa479f8

I wrote this up in the Jest thread tracking the problem: https://github.com/facebook/jest/issues/9771#issuecomment-841624042 and other people have iterated on it for other usecases further down in that thread. From the volume of traffic in that thread I suspect this is going to slowly become a defacto for Jest setups.

achingbrain commented 3 years ago

Ah, that's interesting, @ipld/js-car copies everything to the dist folder and builds types from the generated sources. multiformats takes a different approach, it builds types from the src dir, then copies the types into the dist folder. I think probably better not to build types from generated sources - I've opened https://github.com/ipld/js-car/pull/40 which should sort that out.

achingbrain commented 3 years ago

Also, yeah, it would be better if jest supported exports and this was reverted, though best if browserify-resolve supported exports and #13 was reverted too as that would solve the problem at the root without everyone needing to maintain extra config.

It looks like Jordan wanted some help fleshing out https://github.com/ljharb/list-exports as a test case?