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

9.1.7 => 9.2: can't build with TypeScript - TS1471: Module 'mermaid' cannot be imported using this construct #3747

Closed
slorber opened this issue Nov 2, 2022 · 3 comments · Fixed by #3774
Labels
Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect

Comments

@slorber
Copy link

slorber commented Nov 2, 2022

Describe the bug

Docusaurus introduced Mermaid support last week: https://docusaurus.io/blog/releases/2.2

On 9.1.7 it works fine, but on 9.2.0 it fails, and users installing our Mermaid plugin started to report failures: facebook/docusaurus#8274

To Reproduce
Steps to reproduce the behavior:

  1. clone https://github.com/facebook/docusaurus
  2. try to force upgrade dependency in packages/docusaurus-theme-mermaid
  3. run yarn workspace @docusaurus/theme-mermaid build
src/client/index.ts:10:21 - error TS1471: Module 'mermaid' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported synchronously. Use dynamic import instead.

10 import mermaid from 'mermaid';
                       ~~~~~~~~~


Found 1 error.

I'm able to make it compile by removing the exports "types": "./dist/mermaid.d.ts"

And it seems to work at runtime as before, but I still get those Webpack console warnings:

index.js?e3e2:556 [webpack-dev-server] WARNING in ../node_modules/mermaid/dist/mermaid.core.mjs 27645:52-67
Critical dependency: the request of a dependency is an expression
logger @ index.js?e3e2:556
eval @ index.js?e3e2:730
warn @ index.js?e3e2:189
warnings @ index.js?062c:247
eval @ socket.js?7b31:60
client.onmessage @ WebSocketClient.js?e23c:50
index.js?e3e2:556 [webpack-dev-server] WARNING in ../node_modules/mermaid/dist/mermaid.core.mjs 27659:52-63
Critical dependency: the request of a dependency is an expression

Note: I did try to remove the DefinitivelyTyped bindings, and it doesn't improve anything.

Expected behavior

It works as before for library consumer apps OR the major version gets bumped for significant changes that might affect consumer apps. We rely on ^ so our users auto-upgrade to the latest 9.x.

I'm not sure what exactly changed in this release but it looks like it's a quite significant one (monorepo, TS...), and was wondering if it wouldn't be safer to release all this in v9.x instead of v8.x.

@slorber slorber added Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect labels Nov 2, 2022
@slorber slorber changed the title 9.2 can't build with TS anymore 9.1.7 => 9.2: can't build with TypeScript - TS1471: Module 'mermaid' cannot be imported using this construct Nov 2, 2022
@aloisklink
Copy link
Member

I'm not really 100% sure why this issue is occurring. From what I can see, the v9.2.0 Mermaid package.json file looks exactly the same as the recommended TypeScript package.json file for a dual ESM/CommonJS package, see

"type": "module",
"exports": {
".": {
"require": "./dist/mermaid.min.js",
"import": "./dist/mermaid.core.mjs",
"types": "./dist/mermaid.d.ts"
},
"./*": "./*"
},

and

    "type": "module",
    "exports": {
        ".": {
            // Entry-point for `import "my-package"` in ESM
            "import": "./esm/index.js",
            // Entry-point for `require("my-package") in CJS
            "require": "./commonjs/index.cjs",
        },
    },

From https://www.typescriptlang.org/docs/handbook/esm-node.html#packagejson-exports-imports-and-self-referencing

However, switching to "type": "commonjs" seems to have fixed the issue, see #3767.


After fixing that issue, there was also an incorrect type error in mermaid.render() that I also had to fix (#3768).

After that, docusaurus builds fine, yarn test passes, and running yarn start and visiting http://localhost:3000/docs/markdown-features/diagrams seemed to work fine!

I'm not sure what exactly changed in this release but it looks like it's a quite significant one (monorepo, TS...), and was wondering if it wouldn't be safer to release all this in v9.x instead of v8.x.

As far as I'm aware, everything was supposed to be backwards compatible, but clearly we could use better tests.
@knsv, is it worth the mermaid project having a documented release process (e.g in the CONTRIBUTING.md file), which includes doing tests on major dependents like docusaurus?

@sidharthv96
Copy link
Member

Can you please test 9.2.2-rc.2

@slorber
Copy link
Author

slorber commented Nov 10, 2022

Thanks for your reactivity

I can confirm 9.2.2 works fine now 👍

Not sure exactly what is happening either 😅

@slorber slorber closed this as completed Nov 10, 2022
This was linked to pull requests Nov 10, 2022
This was unlinked from pull requests Nov 10, 2022
@sidharthv96 sidharthv96 linked a pull request Nov 10, 2022 that will close this issue
3 tasks
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
Development

Successfully merging a pull request may close this issue.

3 participants