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

remove baseUrl from tsconfig and fix imports #2191

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adamhl8
Copy link

@adamhl8 adamhl8 commented Dec 20, 2023

Currently, this project uses absolute imports which breaks module resolution when developing against the package. That is, if you do something like this import { DataviewInlineApi } from "obsidian-dataview/lib/api/inline-api", TypeScript can't resolve the absolute imports in inline-api.d.ts. This is because those imports are only valid when baseUrl is set in the tsconfig (which is not included in the published package).

This PR removes baseUrl from tsconfig and changes all relevant imports to be relative. TypeScript itself recommends not using baseUrl:

This feature was designed for use in conjunction with AMD module loaders in the browser, and is not recommended in any other context.

For more context, this StackOverflow post is a good explanation as well.

Currently, all tests pass except index-map.test.ts. It's failing because of the non-standard web-worker: syntax in import-manager.ts: Cannot find module 'web-worker:./import-entry.ts' from 'src/data-import/web-worker/import-manager.ts'

I'm not super familiar with rollup or the web-worker-loader plugin, so I'm not sure what the best way to fix that is (maybe something needs to be done on the jest side?)

@blacksmithgu
Copy link
Owner

Hmm, I was not aware of this baseUri behavior. This seems a bit strange though... is there actually no way to do absolute imports inside of a library package? It seems weird to require all imports be relative.

@adamhl8
Copy link
Author

adamhl8 commented Dec 31, 2023

You can use paths to accomplish more or less the same thing. And it doesn't affect the emitted import paths so it should work fine in this case.

@GottZ
Copy link
Contributor

GottZ commented Mar 25, 2024

now that I read this, there is actually some truth to it.
before I started actually using this codebase, I've ran tsc -d --declarationDir dist/lib --declarationMap --emitDeclarationOnly to generate myself some juicy docs for vscode for dev on my views.
I had to manually tamper with the import paths in each file to make it actually work because of the above mentioned issue.

@Mara-Li
Copy link

Mara-Li commented Apr 6, 2024

This will fixe #2080

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 this pull request may close these issues.

4 participants