Jest ignoring browser field in package.json of a module dependency sub-directory

[jasonmacdonald]

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 a file, which passes, so the loadAsDirectorySync 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
Node v8.6.0
Yarn 1.1.0

 "jest": {
        "coveragePathIgnorePatterns": [
            "/node_modules/",
            "/mocks/",
            "<rootDir>/scripts/test.setup.js"
        ],
        "coverageThreshold": {
            "global": {
                "branches": 0,
                "functions": 0,
                "lines": 0,
                "statements": 0
            }
        },
        "bail": true,
        "browser": true,
        "setupFiles": [
            "<rootDir>/scripts/test/setup.js"
        ],
        "testPathIgnorePatterns": [
            "/mocks/"
        ],
        "moduleFileExtensions": [
            "js"
        ],
        "moduleDirectories": [
            "node_modules"
        ],
        "moduleNameMapper": {
            "\\.(jpg|jpeg|png|svg|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$": "<rootDir>/scripts/test/fileMock.js",
            "\\.(css|scss|sass)$": "identity-obj-proxy"
        },
        "transform": {
            "^.+\\.js$": "babel-jest",
            "^.+\\.(html|marko)$": "<rootDir>/scripts/test/marko.js"
        },
        "transformIgnorePatterns": [
            "node_modules/(?!@lwc-.+)"
        ]
    },

[jasonmacdonald]

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.

[SimenB]

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

[SimenB]

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

[jasonmacdonald]

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.

[jasonmacdonald]

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.

[jasonmacdonald]

So it seems there are a couple of issues at play.

1. The 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 *
2. The algorithm used in 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.
3. The logic to find the "shims" (as they called it) is done upfront, before resolving the path, so unless you force the basedir to start from the fully resolved file, it will resolve the file and simply return, never seeing the package.json which held the browser "shims".

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.

[DylanPiercey]

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.

[github-actions]

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.

[github-actions]

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.

[github-actions]

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.