Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The resolve process does not report an error when the only main field of the packagejson points to a non-existent file. #162

Closed
zhusjfaker opened this issue May 20, 2024 · 8 comments

Comments

@zhusjfaker
Copy link

When the only main field of the packagejson points to a non-existent file, the resolve process does not report an error and continues to look for 'index.js' by default.

rust code case:

https://github.com/zhusjfaker/oxc-resolver-has-issue-in-esbuild-test-case-packagejson-2002b/blob/fix/main_fields_not_resolve_case/tests/mod.rs#L29

rust repo and branch:

https://github.com/zhusjfaker/oxc-resolver-has-issue-in-esbuild-test-case-packagejson-2002b/tree/fix/main_fields_not_resolve_case

cmd:
cargo test

    #[test]
    fn test_resolver_package_json_bad_main() {
        let options = ResolveOptions {
            alias_fields: vec![vec!["browser".into()]],
            exports_fields: vec![vec!["exports".into()]],
            condition_names: vec!["node".into(), "import".into(), "require".into()],
            main_fields: vec!["main".into()],
            ..ResolveOptions::default()
        };
        let dir = path_resolve("test_package_json_bad_main/src");
        let resolver = Resolver::new(options);
        let res = resolver.resolve(dir, "demo-pkg").unwrap();
        println!("res-> \n {:#?}", res);
        /*
        node_modules/demo-pkg/package.json
        {
          "main": "./does-not-exist.js"
        }
        the main field in package.json is invalid
        but oxc_resolver will try to resolve index.js
         */
    }
@zhusjfaker
Copy link
Author

terminal now

running 1 test
test tests::test_resolver_package_json_bad_main ... ok

successes:

---- tests::test_resolver_package_json_bad_main stdout ----
res-> 
 Resolution {
    path: "/Users/jason.zhu/Desktop/rs-test/resovler/test_package_json_bad_main/node_modules/demo-pkg/index.js",
    query: None,
    fragment: None,
    package_json: Some(
        "/Users/jason.zhu/Desktop/rs-test/resovler/test_package_json_bad_main/node_modules/demo-pkg/package.json",
    ),
}


successes:
    tests::test_resolver_package_json_bad_main

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

expect throw a panic

@zhusjfaker

This comment was marked as spam.

@ematipico
Copy link
Collaborator

@zhusjfaker you've been asked to not @ people unless it's urgent. Please, don't be that person.

@Boshen
Copy link
Member

Boshen commented May 20, 2024

cjs require("demo-pkg"); throws a deprecated warning:

(node:18447) [DEP0128] DeprecationWarning: Invalid 'main' field in '/Users/boshen/oxc/oxc-resolver-has-issue-in-esbuild-test-case-packagejson-2002b/test_package_json_bad_main/node_modules/demo-pkg/package.json' of './does-not-exist.js'. Please either fix that or report it to the module author

esm (import * as x from "demo-pkg"; passes.

oxc-resolver's behaviour is correct according to both algorithms specified in

When you say it's incorrect, please base evidence from the specification instead speculating, to save us some time. Thank you.

@Boshen Boshen closed this as not planned Won't fix, can't repro, duplicate, stale May 20, 2024
@zhusjfaker
Copy link
Author

@zhusjfaker you've been asked to not @ people unless it's urgent. Please, don't be that person.

sorry

@zhusjfaker
Copy link
Author

cjs require("demo-pkg"); throws a deprecated warning:

(node:18447) [DEP0128] DeprecationWarning: Invalid 'main' field in '/Users/boshen/oxc/oxc-resolver-has-issue-in-esbuild-test-case-packagejson-2002b/test_package_json_bad_main/node_modules/demo-pkg/package.json' of './does-not-exist.js'. Please either fix that or report it to the module author

esm (import * as x from "demo-pkg"; passes.

oxc-resolver's behaviour is correct according to both algorithms specified in

When you say it's incorrect, please base evidence from the specification instead speculating, to save us some time. Thank you.

I understand, sorry for the intrusion, but oxc-resolver looks inconsistent with esbuild-resolver in some places so far. I need to go back to rolldown to discuss this, and the next time I submit an isuee, I will strictly refer to esm to ensure that the tests are comprehensive before submitting the correct isuee.

@Boshen
Copy link
Member

Boshen commented May 21, 2024

inconsistent with esbuild-resolver

Please report the expected result if this is the case.

@zhusjfaker
Copy link
Author

inconsistent with esbuild-resolver

Please report the expected result if this is the case.

sorry i got misstake , The current results are correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants