New bundling strategy makes it impossible to debug and apply patches

[andrico1234]

Hi folks!

Congrats on the 4.0.0 release 🥳 🎈

We've been upgrading our codebase, and we're coming across a handful of issues. We've managed to get to the bottom of most of them, but we're no longer able to debug in the same way that we were able to previously.

I believe that this is result of two significant changes:

Issue 1

First, the React Admin output is minified.

In v3, the code was transpiled down from TS to JS,with little or no other code transformation being applied. Since v4 the code is also being minified, which causes a lot of problems with debugging.

No longer can we jump into our node_modules and run debuggers and logs to observe what's happening with our code. It also means that it's impossible to use tools like patch-package to make any small bug fixes that may not be addressed until future versions of RA.

I believe that minification is an application-level concern, and library authors shouldn't need to perform this step. It's up to developers to decide on how they want to bundle their code for deployment. It makes a lot of sense to revert this change, though I'm also interested to hear what the benefits are of this new approach.

Issue 2

The other issue, which may have just been a little oversight is that we're no longer able to click on an export to get directed to the source code, instead, it's reverted to taking us to the types again.

I don't think that I'll be able to replicate this via a CodeSandbox, but I'll whip up a before/after video

Before

After

Shows what the output of the ESM bundle looks like

Edit:

Removing --minify from the build script prevents the output from being minified, which is good, but keeps the entire output contained in a single index.js file. Are there code splitting concerns to be had with compiling the entire output into a single file?

Other information:

Environment

• React-admin version:
• Last version that did not exhibit the issue (if applicable):
• React version:
• Browser:
• Stack trace (in case of a JS error):

[djhi]

Those are good points, thank you for reporting them. I'll discuss with the team to see what we can do

[andrico1234]

Thanks! A real-life example of how this would be useful is that we're not able to deploy our upgraded application until this particular PR is resolved.

In the past, we've been able to use patch-package to make the changes locally and remove the patch once the version that includes the fix is released. Unfortunately, we can't do this anymore since the output is minified.

[djhi]

FYI, the src directory is still included in the packages. Can't you configure your bundler and patch-package to use it?

[andrico1234]

I figure that's not too difficult to do. I haven't tried it, but it seems feasible. I'll let you know how I get on!

That doesn't resolve some of the other issues with debugging too. Being able to drop in console.logs and debugger statements in the esm file without having to rebuild packages has been really handy when troubleshooting.

I also tried following the instructions for using yarn link to link packages, but it seems that with the repo's upgrade to yarn v3 the instructions are out of date.

[slax57]

Btw @andrico1234 , after some digging and testing, I think we need to bring some additional clarifications about the way we fixed sourcemaps.
In the end it appears that PR #7803 , while certainly improving the build config, does not solve the sourcemap issue on its own.

It appears that this issue is rather dependent on the web packager (webpack, vite, ...) configuration, rather than the tool we use to build the packages (tsup or tsc).

For example, we had the same issue in our storybook for enterprise modules, and we fixed it by adding this in the webpack config:

module.exports = {
    [...]
    webpackFinal: async (config, { configType }) => {
        if (configType === 'PRODUCTION') {
            [...]
+       } else {
+           config.module.rules.unshift({
+               test: /\.js$/,
+               enforce: 'pre',
+               use: ['source-map-loader'],
+           });
        }

        return addReactAdminAliases(updateEmotionAliases(config));
    },
};

Hope this helps!

[slax57]

Coming back to this issue, again 😅 , just to let you know @andrico1234 that @fzaninotto did some more testing, and it turns out tsup actually does have a role to play in this issue after all.
We will probably have to revert back to using tsc again, for the reasons he explains here. And this will probably have an impact on this issue as well.
Anyways, just wanted to let you know we are not yet done on this matter, and we're still working on a solution (hopefully we'll figure one out! 🙂 )
Cheers