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

fix: Resolve relative dependencies before creating imports from them #507

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

codeworrior
Copy link
Member

TypeScript cannot recognize that a relative dependency "./library" in a module "sap/ui/unified/ColorPicker" maps to the ambient module "sap/ui/unified/library" (without import maps / package.json entries, relative import specifiers don't map to any bare modules).

By resolving relative dependencies before creating imports from them, this can be improved. As the resolution is no longer unique when "resource-roots" configuration is used, the resolution is limited to productive code. in test code, the use of "resource-roots" mappings is much more likely.

TypeScript cannot recognize that a relative dependency "./library"
in a module "sap/ui/unified/ColorPicker" maps to the ambient module
"sap/ui/unified/library" (without import maps / package.json entries,
relative import specifiers don't map to any bare modules).

By resolving relative dependencies before creating imports from them,
this can be improved. As the resolution is no longer unique when
"resource-roots" configuration is used, the resolution is limited to
productive code. in test code, the use of "resource-roots" mappings is
much more likely.
@codeworrior
Copy link
Member Author

The failing tests show that there's a trade-off for this PR:

  • ambient modules are now taken into account and some more issues can be detected that way
  • on the other side, information from modules that are not described as ambient modules but used via relative imports is now lost (E.g. ./Metadata in the sap/ui/base/Objectin the sap.ui.core fixture project).

@codeworrior codeworrior marked this pull request as draft January 29, 2025 18:26
@codeworrior
Copy link
Member Author

Maybe we should experiment with TS compiler option paths (https://www.typescriptlang.org/tsconfig/#paths). It should allow TS to find a file for a missing bare module.

@d3xter666
Copy link
Contributor

Maybe we should experiment with TS compiler option paths (https://www.typescriptlang.org/tsconfig/#paths). It should allow TS to find a file for a missing bare module.

Paths seem like a promising option, but In the end it might still not be able to resolve the issue completelly.
For example, if we analyze the whole openui5 repo, then there might be numerous colliding "./library" imports that are actually libraries from different namespaces- sap.m, sap.ui.core, etc.

Maybe, we need to strive for some dynamic behaviour where the relative path resolve is based on current file/moule namespace and is available in TS only for this file/module analysis, so we don't polute TS definitions with relative paths.
Not quite sure how to achieve that for now though...

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.

2 participants