Should v6 maintain compatibility with React 17?

[ignatiusreza]

I'm opening this issue to confirm if the intention is for react-querybuillder to maintain compatibility with React 17. If yes, then it needs to be fixed. If not, then I would suggest to add peerDependencies entry pointing the desired compatibility matrix..

The release of v6 (specifically this commit) breaks compatibility with React 17 with the following error

ERROR in ./node_modules/react-querybuilder/dist/index.mjs 1:0-73
Module not found: Error: Can't resolve 'react/jsx-runtime' in './node_modules/react-querybuilder/dist'
Did you mean 'jsx-runtime.js'?
BREAKING CHANGE: The request 'react/jsx-runtime' failed to resolve only because it was resolved as fully specified
(probably because the origin is strict EcmaScript Module, e. g. a module with javascript mimetype, a '*.mjs' file, or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.

The issue is caused by esm module trying to include non-esm file with no extension.
It is not an issue in react 18 because of this PR which added exports entry within react package.json which map jsx-runtime to jsx-runtime.js..

meanwhile, those facing the same issue can add the following webpack plugin to fix their build

/*
 * Fix compatibility of react-querybuilder with React 17
 *
 * ref: https://github.com/react-querybuilder/react-querybuilder/issues/464
 */
new NormalModuleReplacementPlugin(
  /react\/jsx-runtime/,
  require.resolve('react/jsx-runtime')
),

[jakeboone02]

Thanks for this detailed report! It was definitely not my intention to remove React 17 compatibility, but the v6 build is quite different so I'm not surprised there are issues.

The change in ffc0f5f was actually just to remove jsx-runtime from the build output, specifically the dev version which was adding significantly to the bundle size. Naïve and short-sighted of me to do that without testing React 17!

Would you happen to know the best way to fix the issue (here in the library of course, not a user-land workaround...although thanks for that webpack tip)? I'll start investigating today but if you have any advice, please share.

[jakeboone02]

OK so I've tried RQB + React 17 with a few different build systems on CodeSandbox and I'm not seeing any issues:

• CRA 4 https://codesandbox.io/s/pl2yf5
• CRA 5 https://codesandbox.io/s/x7fbp0
• Vite 4 https://codesandbox.io/s/2qu662
• Parcel 2 https://codesandbox.io/s/8v89w7

Would you mind creating a minimal repo that reproduces the issue?

[ignatiusreza]

https://github.com/ignatiusreza/react-querybuilder-464/pulls

I'm not quite sure on how to use codesandbox, so I created a reproduction repo. It has 2 PRs, one with successful build when using v5.4.1, and another one with failing build when using v6.0.0.

It could have been the case that it's a webpack only issue.. 😅

[ignatiusreza]

The following patch fix the build, since it tell webpack to use the cjs build instead of the esm build.. AFAIK, only webpack make use of the development & production condition in exports..

diff --git a/packages/react-querybuilder/package.json b/packages/react-querybuilder/package.json
index 18c8a955..ffd55fc3 100644
--- a/packages/react-querybuilder/package.json
+++ b/packages/react-querybuilder/package.json
@@ -7,6 +7,14 @@
   "exports": {
     "./package.json": "./package.json",
     ".": {
+      "development": {
+        "types": "./dist/types/index.d.ts",
+        "default": "./dist/index.cjs"
+      },
+      "production": {
+        "types": "./dist/types/index.d.ts",
+        "default": "./dist/index.cjs"
+      },
       "import": {
         "types": "./index.d.mts",
         "default": "./dist/index.mjs"

or

diff --git a/packages/react-querybuilder/package.json b/packages/react-querybuilder/package.json
index 18c8a955..929a697c 100644
--- a/packages/react-querybuilder/package.json
+++ b/packages/react-querybuilder/package.json
@@ -7,6 +7,10 @@
   "exports": {
     "./package.json": "./package.json",
     ".": {
+      "webpack": {
+        "types": "./dist/types/index.d.ts",
+        "default": "./dist/index.cjs"
+      },
       "import": {
         "types": "./index.d.mts",
         "default": "./dist/index.mjs"

[jakeboone02]

Thanks for the repo! That's perfect. I've seen a similar error with Docusaurus, and putting this in the webpack.config.js seemed to resolve it:

{
  "module": {
    "rules": [
      {
        "test": /\.m?js$/,
        "resolve": {
          "fullySpecified": false
        }
      }
    ]
  }
}

[ignatiusreza]

That also does the trick 👍

Would you suggest than for the path forward to be in documenting the needed configuration?

[jakeboone02]

I think I'll just direct people to this issue if it comes up again. Otherwise, they can find the answer through a search since the error message is in your original comment.

[jakeboone02]

@ignatiusreza - I know we closed this issue since the workaround in #464 (comment) was acceptable, but if you get a chance could you try using v6.3.0 without the workaround? I have a feeling that v6.3.0 might resolve the issue for good.

[ignatiusreza]

You're right! v6.3.0 works without the workaround 👍