search

nodejs / import-in-the-middle

Like `require-in-the-middle`, but for ESM import
https://www.npmjs.com/package/import-in-the-middle
Apache License 2.0
60 stars 22 forks source link

Fix node 20 reexports #30

Closed luxaritas closed 7 months ago

luxaritas commented 1 year ago

Resolves #29

tlhunter commented 1 year ago

@luxaritas I see this is a draft... Is it ready to review?

luxaritas commented 1 year ago

Hey @tlhunter - main reason I left it as a draft is because I'm not confident that my approach here is "correct" - particularly how I'm handling path resolution seems hacky. That said I don't have any pending work on it, really just waiting for input, so I'll mark it as ready for review

luxaritas commented 9 months ago

@trentm Thanks for the patch - looks reasonable to me so I've applied it. (I've marked you as a commit coauthor using git metadata from a recent GitHub commit of yours, let me know if that should be something else!)

jsumners-nr commented 8 months ago

If I'm not mistaken, this is solved by #43.

pichlermarc commented 8 months ago

If I'm not mistaken, this is solved by #43.

Hmm, I I've tried the fix from #43 (released in 1.7.2) and I think it does not cover the case of CJS re-exports.

jsumners-nr commented 8 months ago

If I'm not mistaken, this is solved by #43.

Hmm, I I've tried the fix from #43 (released in 1.7.2) and I think it does not cover the case of CJS re-exports.

Drat!

trentm commented 8 months ago

This is just speculation, based on commit bb038e9 of this PR handling a circular imports issue: I wonder if this PR would also have resolved https://github.com/DataDog/import-in-the-middle/issues/31 I haven't yet had a chance to understand what #43 is doing.

trentm commented 7 months ago

Woot. Thanks very much.