Update resolve dependency to 1.12.0 #93
Comments
|
I'm working on this.
|
|
Hi, |
|
New failures are: |
|
A newer version of resolve is also needed to be compatible with yarn 2's Plug'n'Play default linking strategy. https://next.yarnpkg.com/advanced/migration#make-sure-you-use-resolve19 |
|
@guimard please let me know if there's still test failures on the latest version of resolve. |
|
There are seven fails at the moment: |
|
@ljharb I investigated a bit and the regression started in 1.7.0 because of browserify/resolve#152 Reverting the changes causes the tests to pass (even in 1.15.1). I'm not sure what that entails though, or whether the problem should be fixed by |
|
Hmm - the implication is indeed that browser-resolve is relying on the bug that that PR fixed :-/ To be honest, though, some of those tests (1, 6, 7) just look like they're testing unimportant info - ie, it's like a snapshot test, where you don't actually care what the value is, but you've still asserted on it anyways. Perhaps those assertions can either be updated, or removed, or perhaps I'm missing something vital? The other 4 tests look important, though - they're all passing in |
|
Hi, This (revert) patch fixes browser-resolve tests without changing resolve tests: --- a/lib/async.js
+++ b/lib/async.js
@@ -265,13 +265,13 @@
function isdir(err, isdir) {
if (err) return cb(err);
if (!isdir) return processDirs(cb, dirs.slice(1));
- loadAsFile(dir, opts.package, onfile);
+ loadAsFile(dir, undefined, onfile);
}
function onfile(err, m, pkg) {
if (err) return cb(err);
if (m) return cb(null, m, pkg);
- loadAsDirectory(dir, opts.package, ondir);
+ loadAsDirectory(dir, undefined, ondir);
}
function ondir(err, n, pkg) { |
|
@guimard well sure, but that reverts a bugfix in resolve. The fix for browser-resolve is likely going to involve changing browser-resolve. |
|
Any news on this? Yarn 2 PnP usage depends on this update. |
|
I think I updated tests correctly for browser-resolve in https://github.com/browserify/browser-resolve/compare/resolve-twelve, but there are a lot of test failures in browserify with that change that will need more investigation. |
|
@goto-bus-stop can you link me to the browserify test failures? |
|
The majority actually seem to be caused by #92. If there are any remaining ones not related to that i'll push a branch to browserify and let you take a look 😇 |
|
#92 is made obsolete by resolve's
(it's getting late here so I'll call it quits for today!) |
|
hmm - nothing jumps out at me. if that's the only failure tho, then could you try bisecting a bit with resolve versions? |
|
Finally took some time to dive into this. The failure starts at resolve 1.7.0 with the Now when @ljharb Is that the intended behaviour for |
|
@goto-bus-stop thanks for the research! Is the bugfix you're referring to browserify/resolve#152? The original creation of |
|
Right, yes, browserify/resolve#152. I'll publish this as a major version then so older browserify versions don't auto-upgrade. |
The following tests fail with resolve 1.12.0
The text was updated successfully, but these errors were encountered: