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

Some typescript import errors around missing files #5747

Closed
bollwyvl opened this issue Aug 23, 2024 · 7 comments · Fixed by #5805 or #5838
Closed

Some typescript import errors around missing files #5747

bollwyvl opened this issue Aug 23, 2024 · 7 comments · Fixed by #5805 or #5838
Labels
Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect

Comments

@bollwyvl
Copy link
Contributor

Description

Congratulations on 11.0.0 🎉 !

When building running tsc on some code that imports mermaid

  ../../node_modules/mermaid/dist/rendering-util/render.d.ts(1,26): error TS2307: Cannot find module '$root/diagram-api/types.js' or its corresponding type declarations.
  ../../node_modules/mermaid/dist/rendering-util/render.d.ts(2,38): error TS2307: Cannot find module '$root/internals.js' or its corresponding type declarations.
  ../../node_modules/mermaid/dist/rendering-util/render.d.ts(3,33): error TS2307: Cannot find module './types.js' or its corresponding type declarations.
  ../../node_modules/mermaid/dist/mermaid.d.ts(10,33): error TS2307: Cannot find module './rendering-util/types.js' or its corresponding type declarations.

Steps to reproduce

Here is a minimal reproducer, but can confirm by inspection rendering-utils/types.d.js is not present in https://registry.npmjs.org/mermaid/-/mermaid-11.0.0.tgz . The $root stuff looks plain wrong.

Screenshots

No response

Code Sample

https://gist.github.com/bollwyvl/00b49e5119fe8115c2bf606e5c669a31

Setup

  • Mermaid version: 11.0.0
  • Browser and Version: [Chrome, Edge, Firefox] (havne't gotten here yet)

Suggested Solutions

  • include the missing .d.ts files
  • see if the $root pattern can be replaced with
    • more-consistent (albeit verbose) relative imports, or
    • further package splitting

Additional Context

Adding skipLibCheck makes this "work," but is infeasible for many projects.

@bollwyvl bollwyvl added Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect labels Aug 23, 2024
@bollwyvl
Copy link
Contributor Author

This issue persists with the latest releases.

bollwyvl added a commit to bollwyvl/mermaid that referenced this issue Aug 30, 2024
@bollwyvl
Copy link
Contributor Author

#5798 addresses the missing file part of this.

It appears the paths: {$root} alias in tsconfig.json is apparently intractable to solve use stock tsc and still provide useful types to downstreams without further configuration.

Options to fix this would would include:

  1. replace the ~100 instances with their relative paths, which get are properly understood by all tooling
  2. use a downstream-transparent pattern, e.g.
"paths": {"mermaid/dist/*": ["src/*"]}
  1. use a tsc wrapper (there are several, but I can't really recommend any of them)
  2. use a post-processor (similar to the above)

While the second option appears to work, as a downstream consumer I'd really favor the first option, as anything else is liable to become more brittle over time.

However this is solved, a CI job that does a full build of the .tgz files and does something akin to the above-linked gist would help ensure the library stays consumable by downstreams.

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Sep 1, 2024

#5805 offers a full, tested proposal, implementing option 2 from the above comment.

bollwyvl added a commit to bollwyvl/mermaid that referenced this issue Sep 2, 2024
bollwyvl added a commit to bollwyvl/mermaid that referenced this issue Sep 4, 2024
bollwyvl added a commit to bollwyvl/mermaid that referenced this issue Sep 4, 2024
bollwyvl added a commit to bollwyvl/mermaid that referenced this issue Sep 8, 2024
bollwyvl added a commit to bollwyvl/mermaid that referenced this issue Sep 8, 2024
github-merge-queue bot pushed a commit that referenced this issue Sep 8, 2024
bollwyvl added a commit to bollwyvl/mermaid that referenced this issue Sep 8, 2024
github-merge-queue bot pushed a commit that referenced this issue Sep 9, 2024
fix 5747 replace $root with relative paths
@sidharthv96
Copy link
Member

@bollwyvl it's out in v11.2.0!
Thanks for the effort.

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Sep 9, 2024

Thanks. Perhaps a @mermaid-js/layout-elk which includes the #5798 typing changes would also be needed?

@sidharthv96
Copy link
Member

Ohh, didn't notice there was no changeset for that.
Will do.

@sidharthv96
Copy link
Member

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
2 participants