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

Module resolution improvements #60

Merged

Conversation

Daniel-Knights
Copy link
Contributor

@Daniel-Knights Daniel-Knights commented Sep 4, 2024

This PR adds support for resolving module paths using explicit module ids (@module <id>) and implicit module ids, mirroring the same method JSDoc uses. The typescript.moduleRoot config option is still prioritised over these for backwards compact.

This PR also:

  • Removes leading ../ path segments from module ids (if a referenced module is located outside the cwd), as they cause errors.
  • Resolves /index.js for paths that target the parent directory.

Changes

  • Update README
  • No longer throw errors for missing typescript.moduleRoot config option, only if present and the provided path doesn't exist
  • Use absolute file paths as keys in moduleInfos and fileNodes instead of module ids to simplify setting and accessing
  • getImplicitRoot function for when there's no typescript.moduleRoot option or explicit module id
  • getModuleId function
  • Cache module ids in moduleInfos
  • Remove parser param from getDefaultExportName and getDelimiter as it's being passed to the extension param of getModuleInfo

Additional notes

In my opinion, the typescript.moduleRoot config option should be deprecated, as it introduces the risk of a mismatch between what JSDoc considers an existing module id and the id this plugin uses internally.

I was unsure how to add tests for this - open to any suggestions.

index.js Outdated Show resolved Hide resolved
Copy link
Member

@ahocevar ahocevar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to remove the moduleRoot config option, and we can publish a major release with that breaking change. No need to deprecate.

@Daniel-Knights
Copy link
Contributor Author

Okay, I've removed typescript.moduleRoot

Copy link
Member

@ahocevar ahocevar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @Daniel-Knights

@ahocevar ahocevar merged commit e07b6d4 into openlayers:main Sep 13, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants