Closed jasonmacdonald closed 2 years ago
Update... I may have jumped the gun. It seems Jest is using a third party module for browser-resolution. I think it may be the one not handling this use-case. I'll leave the ticket up for now as I investigate further, but others may run into this same issue. Not sure what the stance is on bugs related to third party modules that are used by Jest.
Can you create a small repro repo? I use marko at work myself, but only on the server-side, so I haven't hit this case before.
But, I believe the issue is that the
sync.js
file in Jest, on line 24,
There is no file called sync
in the jest source tree
It seems Jest is using a third party module for browser-resolution. I think it may be the one not handling this use-case.
That doesn't sound right either, jest uses jest-resolve
, https://github.com/facebook/jest/tree/master/packages/jest-resolve
EDIT: Ah, for the browser-part specifically jest-resolve
has an external dep: https://github.com/facebook/jest/blob/688fba7fe22ed0d904c613cfdb527a0e45992a26/packages/jest-resolve/src/default_resolver.js#L12
https://github.com/defunctzombie/node-browser-resolve
That's what browserify uses as well
Jest-resolve uses browser-resolve
internally, which I thought was the issue but I think I finally found the real issue. It's the basedir
options being used when the file path is relative. In the example above, the require statement is require('../component/utils');
and this sets the options.basedir parameter passed to resolve call as ../marko/dist/runtime
. It is this basedir
that browser-resolve
uses to find package.json files to check for browser properties. If I create my own resolver to replace Jests, include the browser-resolve
package and simply apply a path.dirname(path.resolve(opts.basedir, file))
on the basedir option to normalize it to the basedir of the file itself, and not the directory of the file which is doing the require than everything works.
So, I'm not entriely sure who should be the one to address this, on one hand Jest could normalize the basedir to the directory location of the actual file (accounting for relative paths) or you could say that the browser-resolve
module could also do that normalization as part of it's shim look-up (the part that handles looking for package.json's to find patched "browser" modules/files.
Actually, the more I look at this the more I think it needs to be resolved on the browser-resolve
side, mucking with the basedir for all files causes many other modules to not properly resolve. I cannot see any way to get the shim-look-up to pay attention to the actual directory the requested file is from outside browser-resolve
itself, and it defaults to using basedir which completely ignores the relative path of the file.
So it seems there are a couple of issues at play.
browser-resolve
module uses options.basedir
to create a list of search paths in which it iterates over to find a package.json
file. This basedir
value - passed in by Jest - is always the directory of the file in which the require statement is being executed from. This means if a project file in /src/somepath/myfile.js
is requiring a file from node_modules/module/src/file
, the browser-resolve
will only create search paths starting from /src/somepath
and work up the directory tree from there. NOTE: This makes sense in terms of trying to simply resolve a path to the file, but in the case where ANY file in node_modules could have a remap declared in it's own package.json, not including paths from the source of the file itself means it never sees the modules package.json browser-resolver
to search these paths stops at the very first package.json found, parses it's browser
object, and then returns. So, if the actual file in the node_modules folder has a package.json
which says that the requested file should be changed to something else, browser-resolve
never sees it because it has built its resolve path from the parent file, which would have stopped as soon as it found the projects root /package.json
. In the end, the only way I have managed to get this resolved is to create my own resolver which basically just mimics what jest-resolve
has in its default_resolver
file, but I take the final result from the first resolver
pass, which is the full path to the file, and then I pass it through the resolver a second time using only the file path and the filename
option. This forces the resolver to only focus on that file directly, which means it'll only build it's resolve path starting from that files node_modules
path. The result on the second pass is the new filepath
has applied the proper browser
shim from utils
to utils-browser
. Here's a snippet of what it looks like.
let resolve = require('browser-resolve');
let path = require('path');
module.exports = function(file, options) {
const resv = options.browser ? resolve : require('resolve');
// run the resolver normally to find the full file path
let filepath = resv.sync(file, {
basedir: options.basedir,
extensions: options.extensions,
moduleDirectory: options.moduleDirectory,
paths: options.paths });
// Now, if the path is a Marko path, perform a second pass to get the proper browser mapping
if(file.indexOf('marko') > -1){ // ugly but works
try {
filepath = resv.sync(filepath, {
filename: filepath
});
} catch(e){
console.log("\n ERROR ON SECOND PASS: ", file, filepath, e.message);
}
}
return filepath;
}
Obviously, this is not very efficient to have to resolve the path twice, so for now I've limited the second pass to only paths which match the marko
node_module. I'm sure there's likely a better way to approach this, but it'll likely require a decent amount of updating to browser-resolve
.
Since this issue was made a new marko jest transform and preset was released. This preset overrides the resolver to side step this issue and instead uses enhanced-resolve-jest.
Would be good to rip out the browser-resolve
module here since, aside from the issues inherent in that module, it also pins the resolve
module to a really old version which brings even more issues.
This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.
This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.
bug
What is the current behavior?
I'm using a package called Markojs, this package makes heavy use of the "browser" field in package.json - not just at the root, but throughout the module src hierarchy. Jest does not abide by these file remaps in sub directories, causing errors when trying to test Marko components with Jest. Everything works fine in Node, as the resolver is able to properly resolve the remapped files.
I am setting the "browser" flag on Jest to true, and it appears to be carrying through the proper code execution in Jest. But, I believe the issue is that the
sync.js
file in Jest, on line 24, first tries to load the file as afile
, which passes, so theloadAsDirectorySync
function never fires, and that appears to be the only place where the package.json browser file remap is checked. Since it's a file in a sub directory, and the file does exists (however is not the correct one) the browser package logic is never checked in that directory.For more details, the first file that I am encountering such a failure on is in the https://github.com/marko-js/marko/tree/master/src/components directory on the utils.js file, which as you can see in that directories package.json file should be remapped to the utils-browser.js file.
What is the expected behavior?
Files should be resolved to the files specified in the browser field, regardless of where the package.json lives
Please provide your exact Jest configuration and mention your Jest, node, yarn/npm version and operating system. Jest
20.0.4
Nodev8.6.0
Yarn1.1.0