mirador icon indicating copy to clipboard operation
mirador copied to clipboard

Mirador ES build artifact contains literal check for fixed version 19.0.0 of react-dom and react

Open lutzhelm opened this issue 9 months ago • 5 comments

While trying out ProjectMirador/mirador-share-plugin#74 I stumbled about an issue that needs to be fixed before an M4 release.

In lines 25240 ff of mirador.es.js from the v4.0.0-alpha.16 build, there is a "hardcoded" version check that originates from react-dom:

if (a !== "19.0.0")
        throw Error(
          `Incompatible React versions: The "react" and "react-dom" packages must have the exact same version. Instead got:
  - react:      ` + (a + `
  - react-dom:  19.0.0
Learn more: https://react.dev/warnings/version-mismatch`)
        );

As soon as a different version of react and react-dom than 19.0.0 is used in a dependent project, this check fails. In my case, both react and react-dom had been resolved as 19.1.0 - which would be fine, if there would not be the literal version check in the Mirador build.

The check is no longer present as soon as react-dom/client is added to rollupOptions.external in the vite config, and the dependent plugin is able to use the resulting build, but I do not know if this is the correct approach.

lutzhelm avatar Apr 07 '25 12:04 lutzhelm

The following change to Mirador's vite config would ensure that additionally exported entry points of peer dependencies are actually handled as external:

          external: (id, parentId) => {
            const peers = Object.keys(packageJson.peerDependencies);
            return peers.indexOf(id) > -1
              || peers.find((peer) => id.startsWith(`${peer}/`))
              || id.startsWith('__tests__/')
              || id.startsWith('__mocks__/');
          },

This would currently reduce the ES build from ca. 2.8MB to 2MB, btw.

I was able to immediately use this build to start the mirador-share-plugin, for example. But the vitest tests failed with an error about unsupported directory imports in ES, e.g.

Directory import '/home/lutzhelm/Projects/mirador-share-plugin/node_modules/@mui/material/Paper' is not supported resolving ES modules imported from /home/lutzhelm/Projects/mirador-share-plugin/node_modules/mirador/dist/mirador.es.js

Does anyone have a deeper understanding of vite, vitest and ES support on nodejs and has the chance to take a look?

lutzhelm avatar Apr 09 '25 15:04 lutzhelm

Please add @geourjoa to the repo so we can mention him. He might have a clue.

daxid avatar Apr 10 '25 15:04 daxid

Hi @lutzhelm

I try multiple thing but was not very concluant.

I see react 19.1.0 and 19.0.0 in Mirador package lock

The only way I success to launch test was :

  • Clone Mirador :
    • Do following modification :
      • Package.json : in devdependacies force version of react-dom
        • "react-dom": "^19.1.0",
      • vite.config.js
        • use your rollupOption.external change :
 external: (id, parentId) => {
            const peers = Object.keys(packageJson.peerDependencies);
            return peers.indexOf(id) > -1
              || peers.find((peer) => id.startsWith(`${peer}/`))
              || id.startsWith('__tests__/')
              || id.startsWith('__mocks__/');
          },

I forked mirador-share on https://github.com/TETRAS-IIIF/mirador-share-plugin/ and you can see my changes here :

https://github.com/ProjectMirador/mirador-share-plugin/compare/convert-miradorsharedialog-to-function...TETRAS-IIIF:mirador-share-plugin:convert-miradorsharedialog-to-function

I force aliasing in vitest config on following package : react reduc react-dom react-redux.

It was mainly random try and small intuition but at the end, tests are launching (and failing) :

Image


I explore these ways without success :

  • Use of npm overiding system on mirador package
  • npm dedupe
  • npm list react react-dom

geourjoa avatar Apr 11 '25 10:04 geourjoa

@lutzhelm I got to the point of your error

Directory import '/home/lutzhelm/Projects/mirador-share-plugin/node_modules/@mui/material/Paper' is not supported resolving ES modules imported from /home/lutzhelm/Projects/mirador-share-plugin/node_modules/mirador/dist/mirador.es.js

I am reading this thread and seeing if it can help us: https://github.com/mui/material-ui/issues/45495 Unfortunately this solution was released into MUI 7, and we use MUI 6.

I updated to MUI 7 locally and the issue was fixed in the mirador-share plugin (and other plugins that I tested).

I think we could make a PR for your new externalize syntax and close this particular issue, and then fix the ESM import issue by upgrading to MUI 7. Then we will have to update everything to MUI 7 everywhere, in all plugins. But hopefully that won't be too bad - I tried upgrading to MUI 7 locally in the mirador-share plugin and it was a very minor change.

marlo-longley avatar Apr 11 '25 16:04 marlo-longley

@lutzhelm I believe we can close this now, but please confirm!

marlo-longley avatar Jul 28 '25 20:07 marlo-longley

Yes, this has been fixed via #4139, this is no longer an issue.

lutzhelm avatar Jul 29 '25 07:07 lutzhelm