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

Path mappings to files resolve incorrectly #253

Closed
jneuendorf-i4h opened this issue May 24, 2023 · 6 comments
Closed

Path mappings to files resolve incorrectly #253

jneuendorf-i4h opened this issue May 24, 2023 · 6 comments
Labels
stale Issue has had no activity for a while

Comments

@jneuendorf-i4h
Copy link

jneuendorf-i4h commented May 24, 2023

Hi,

thanks for your package. It's a shame tsc doesn't compile its resolution behavior.

Scenario

In my scenario, I generated JSON files from my TypeScript code (JSON schema from types).
In order to avoid import errors, I configured my path mappings so that JSON are preferred over source files:

{
  "compilerOptions": {
    "rootDir": "src/",
    "baseUrl": "src/",
    "outDir": "dist/",
    "paths": {
      "~/*": ["./*"],
      "@schema/*": ["./schema/*.json", "./schema/*"],
    },
  }
}

The files are structured like this:

src/schema/
├── a.json
└── a.ts

and an import like this: import { definitions } from "@schema/a".

Expected behavior

This setup works fine with TypeScript. Since I think this package aims to mimic TypeScript's resolution mechanism, just at compile time, I think, this setup should also work with your package.

Actual behavior

The JSON file is not resolved. Instead the compiled JS file is resolved.

At first I thought, the reason is that I used --ext incorrectly. But after some debugging, I found that the import is resolved to .../src/schema/*.json/a and then disregarded. This result is produced by https://github.com/benyap/resolve-tspaths/blob/main/src/steps/generateChanges.ts#L143

resolve(aliasPath, importSpecifier.replace(prefix, ""))

In my understand, this code assumes that all aliasPaths are pointing to directories, which is not necessarily true (as described above). I guess replacing "*" in the path instead of appending to it (in case the alias is not a directory), could solve this issue.

(aliasPath) => isDirectory(aliasPath)
    ? resolve(aliasPath, importSpecifier.replace(prefix, ""))
    : aliasPath.replace("*", importSpecifier.replace(prefix, ""))

Let me know what you think. 😉

@benyap
Copy link
Owner

benyap commented May 26, 2023

Hi @jneuendorf-i4h, thanks for your issue and presenting it very clearly!

Normally, I would import a JSON file using the .json to differentiate it from a module:

import { ... } from "@schema/a.json";
// vs
import { ... } from "@schema/a";

I wasn't even aware that you could import a JSON file without using a .json extension 😂 but I tested it and you are right, the Typescript language server seems to be happy with importing a JSON file without .json if you have "./schema/*.json" as a path mapping.

I will note that if you don't have "...*.json" in the path mapping, you cannot import a JSON file without the .json extension. As you pointed out, it looks like Typescript uses * as a wildcard to match directories and files.

I can take a deeper look into your suggestion when I have time - or feel free to open a PR. Initial thoughts are that it looks like it could work, though it will need to be tested thoroughly.

In the interim, is there a reason why you can't import the file with a .json extension? resolve-tspaths will map your paths correctly if you simply do that 😉

@jneuendorf-i4h
Copy link
Author

Hey @benyap, thanks for the quick reply.

I actually ended up, importing the JSON files explicitly (so like import ... from "./a.json"). Therefore, I am not relying on the requested behavior anymore.

You mentioned that such a feature would need to be well tested and I agree. So if the requested behavior is a bit exotic (and most users don't need it), it may not be worth increasing the complexity of the project. However, it would be helpful to state this caveat1 in the README so people know what they're up to.


1 meaning that only file paths can be correctly resolved

@github-actions
Copy link

github-actions bot commented Jul 3, 2023

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Issue has had no activity for a while label Jul 3, 2023
@benyap
Copy link
Owner

benyap commented Jul 3, 2023

Thanks @jneuendorf-i4h for the suggestion, will update the README!

@benyap benyap closed this as completed Jul 3, 2023
@benyap
Copy link
Owner

benyap commented Jul 3, 2023

@all-contributors add @jneuendorf-i4h for documentation

@allcontributors
Copy link
Contributor

@benyap

I've put up a pull request to add @jneuendorf-i4h! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Issue has had no activity for a while
Projects
None yet
Development

No branches or pull requests

2 participants