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

broken nested and relative symlinks #263

Closed
NicholasLYang opened this issue Oct 3, 2024 · 8 comments · Fixed by #284
Closed

broken nested and relative symlinks #263

NicholasLYang opened this issue Oct 3, 2024 · 8 comments · Fixed by #284
Assignees

Comments

@NicholasLYang
Copy link
Contributor

So uhh, this is a bit of a cart and a horse situation. You can extend a tsconfig using a package, e.g.

{
  "extends": "my-package/tsconfig.json"
}

This doesn't seem to work with oxc_resolver, understandably so, since resolving this package import requires...a resolver, which is what we're creating in the first place. But maybe we can do a "best shot" where we use a basic resolver to resolve the extends key?

Thanks!

@Boshen
Copy link
Member

Boshen commented Oct 4, 2024

Can you get me a production?

I tried adding a test in the branch https://github.com/oxc-project/oxc-resolver/tree/extend-package

just example /Users/boshen/oxc/oxc-resolver/fixtures/tsconfig/cases/extends-package-tsconfig foo

gives me the tsconfig

[src/lib.rs:1116:9] &tsconfig = TsConfig {
    root: true,
    path: "/Users/boshen/oxc/oxc-resolver/fixtures/tsconfig/cases/extends-package-tsconfig/tsconfig.json",
    extends: Some(
        Single(
            "tsconfig-index/tsconfig.json",
        ),
    ),
    compiler_options: CompilerOptions {
        base_url: None,
        paths: Some(
            {
                "foo": [
                    "foo.js",
                ],
            },
        ),
        paths_base: "/Users/boshen/oxc/oxc-resolver/fixtures/tsconfig/node_modules/tsconfig-index",
    },
    references: [],
}

which seems to be correct because paths_base is pointing to tsconfig-index (the "extends": "tsconfig-index/tsconfig.json").

@Boshen
Copy link
Member

Boshen commented Oct 4, 2024

If you want to debug yourself:

vec![self.get_extended_tsconfig_path(&directory, tsconfig, s)?]

Resolving extended ts config just reuses the resolver:

oxc-resolver/src/lib.rs

Lines 1193 to 1219 in 376e254

fn get_extended_tsconfig_path(
&self,
directory: &CachedPath,
tsconfig: &TsConfig,
specifier: &str,
) -> Result<PathBuf, ResolveError> {
match specifier.as_bytes().first() {
None => Err(ResolveError::Specifier(SpecifierError::Empty(specifier.to_string()))),
Some(b'/') => Ok(PathBuf::from(specifier)),
Some(b'.') => Ok(tsconfig.directory().normalize_with(specifier)),
_ => self
.clone_with_options(ResolveOptions {
description_files: vec![],
extensions: vec![".json".into()],
main_files: vec!["tsconfig.json".into()],
..ResolveOptions::default()
})
.load_package_self_or_node_modules(directory, specifier, &mut Ctx::default())
.map(|p| p.to_path_buf())
.map_err(|err| match err {
ResolveError::NotFound(_) => {
ResolveError::TsconfigNotFound(PathBuf::from(specifier))
}
_ => err,
}),
}
}

@NicholasLYang
Copy link
Contributor Author

Thanks for the response! I tracked it down. Seems like this call to realpath here is the culprit. It bubbles up an error which short circuits the logic in load_node_modules that's testing the different modules options.

@NicholasLYang
Copy link
Contributor Author

NicholasLYang commented Oct 11, 2024

Would the solution be to recover in this call failing like the fs.read_to_string call above it? Happy to open a PR if that's the case

@Boshen
Copy link
Member

Boshen commented Oct 12, 2024

Happy to open a PR if that's the case

A reproduction would be better for me 😅

@NicholasLYang
Copy link
Contributor Author

I'm still working on a repro, but it appears that the issue is in fact inside realpath. Specifically, the canonicalization code is attempting to resolve a relative symlink, but that symlink is nested inside another directory that is symlinked. Concretely, this line here is incorrect because that path may not be fully canonicalized.

@NicholasLYang
Copy link
Contributor Author

Here's your repro: https://github.com/NicholasLYang/oxc-repro. Let me know if you have any questions!

@Boshen Boshen self-assigned this Oct 18, 2024
@Boshen Boshen changed the title Extending a tsconfig using a package broken nested and relative symlinks Oct 22, 2024
@Boshen Boshen closed this as completed in f2252a8 Oct 22, 2024
@Boshen
Copy link
Member

Boshen commented Oct 22, 2024

@NicholasLYang released in v2.0.0

NicholasLYang added a commit to vercel/turborepo that referenced this issue Oct 22, 2024
### Description

There's a [bug in
oxc-resolver](oxc-project/oxc-resolver#263)
for symlinks that basically breaks tsconfig resolution with pnpm (in a
specific scenario for extending a tsconfig that's a workspace
dependency). Until that's fixed, this PR switches us to a fork that has
a very dumb hot fix

We also add tsconfig loading, either via a parameter or via walking up
the path hierarchy for the closest tsconfig

### Testing Instructions

Added a test based off of [my
repro](https://github.com/nicholaslyang/oxc-repro). Validated that
doesn't work with main branch of oxc-resolver, but does work on my fork
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

Successfully merging a pull request may close this issue.

2 participants