faucet-pipeline-js(x) does not replace process.env.NODE_ENV when bundling an React app

[ngrewe]

Hey folks,

Having spent countless hours crafting webpack configurations, I find the concept of faucet quite appealing and am currently trying to bundle a simple React application. Unfortunately, I can't really wrap my head around how to do it. A reproducing repository of my problem can be found at ngrewe/faucet-pipeline-react.

If I run this through faucet with jsx and esnext set to true, I end up with a bundle that is unusable in the browser because it still contains commonJS require() calls. These calls are from code imported from within my node_modules folder, so it seems that rollup is not making any effort to pull them into the bundle correctly. Interestingly, if I add a require() call to my own code, that will be bundled properly. (I managed to solve this problem, see comment below)

I've also noticed that if I do not include esnext: true in the faucet configuration, all require()d modules are correctly bundled up, but then things trip up on the various places where React uses process.env.NODE_ENV, which remains present in the finished bundle. In the webpack world, that would automatically be defined away according to the --mode switch – how is Rollup/Faucet supposed to handle this? Any pointers would be greatly appreciated.

Thanks,

Niels

[ngrewe]

a bundle that is unusable in the browser because it still contains commonJS require() calls.

I managed to get that under control by excluding all dependencies from transpilation (exclude: ["*"]).

So the remaining problem would be that the NODE_ENV guards are not replaced as I was expecting.

[ngrewe]

This branch has a potential solution for the problem, but I guess you wouldn't want something like this to be enabled unconditionally? I also now see that #118 is kinda related.

[larsrh]

Hi Niels, this is a known problem with React. The process.env.NODE_ENV convention is non-standard and Webpack has special treatment for it that faucet lacks. We can work around this in the faucet world by using the pre-built UMD files that are shipped in React's npm package. The same can be done for various React-adjacent packages.

For reference, I'm pasting an excerpt of a real-world config file for a React application below:

    js: [
        {
            source: "./src/index.tsx",
            target: "./dist/bundle.js",
            typescript: true,
            jsx: true,
            externals: {
                react: "React",
                "react-dom": "ReactDOM",
                "react-router-dom": "ReactRouterDOM",
            },
        },
    ],

    static: [
        {
            source: "./src/index.html",
            target: "./dist/index.html",
        },
        {
            source: "react/umd/react.development.js",
            target: "./dist/react.js",
        },
        {
            source: "react-dom/umd/react-dom.development.js",
            target: "./dist/react-dom.js",
        },
        {
            source: "react-router-dom/umd/react-router-dom.min.js",
            target: "./dist/react-router-dom.js",
        },
    ],

    // ...

Of course, this requires adding some extra <script> tags to the HTML file.

From our PoV, this solution is even preferred over processing React itself through faucet. The reason is that your application code changes more frequently than your library code, so you can leverage HTTP caching better if your bundle does not contain huge dependencies like React. In particular, you could configure your HTTP server to serve React and others with a longer cache period, or use fingerprinting with an indefinite cache period. These measures will improve further page loads.

Having said all that, there are React libraries (like React helmet) that do not provide prebuilt UMD files but use the process.env.NODE_ENV convention. We're working on this and plan to add support for this to the next major version of faucet-pipeline-js.

[ngrewe]

Thanks Lars!

I'm aware of the benefits of extracting library code from the bundle (I've usually been using Webpack's code splitting for this in the past), just had hoped to start quickly and add such optimisations later on. The snippet you included definitely helped getting my minimal example working, so thanks for that as well!

If there is anything I can do to help with supporting the process.env.NODE_ENV convention, please let me know.

Thanks,

Niels

[FND]

Hey Niels, thanks for your feedback and suggested patch.

As pointed out by Lars, this issue has come up in the past, and while I can see why it'd be useful for faucet-js to support this convention, it needs to be an opt-in feature to account for faucet's philosophy:

faucet tries hard to be replaceable and to stay out of your way: We believe that source code should not rely on any particular build tool, but rather be standards-compliant and thus portable across build systems. Naturally this means that project-specific customization is limited, which we consider a useful constraint – though it means that faucet might not be for everyone.

This means that there needs to be a configuration option and dependencies (i.e. @rollup/plugin-replace) need to be moved into an optional meta-package (see pkg directory). Unfortunately, that makes it a little less straightforward than your current patch suggests. In particular, naming strikes me as a bit of a tricky issue, at least I don't have any good ideas on what that config option should look like.

However, it might might actually be easier (and more philosophy-compliant) to turn this into a separate plugin, post-processing bundles without having to worry about previous transformations. Unfortunately though, composing plugins like that is currently not easy; we're working to improve that.

PS: Out of curiosity, your second comment suggests you're using React's already-optimized distribution with exclude, but use NODE_ENV in your own source code as well?

[larsrh]

In larsrh/faucet-pipeline-essugar@9d1cc94, I've (unconditionally) enabled support for this idiom.

faucet-pipeline-essugar was created as an experimental playground for better JSX and TypeScript support via sucrase. Consequently, its footprint is already a smaller than faucet-pipeline-js, so adding the replace plugin isn't at all problematic. It's designed to be a drop-in replacement for faucet-pipeline-js.

@ngrewe, I've sent a PR to your repo (ngrewe/faucet-pipeline-react#1). Please let us know if that works for you. If so, we can consider adding this into a new major release of faucet-pipeline-js. (We're concerned about long-term stability of faucet, so additions like this need to be carefully considered.)

[ngrewe]

Thanks for your feedback. The approach taken by faucet-pipeline-essugar works for me! What is the relation to faucet-pipeline-js? Do you plan to eventually have faucet-pipeline-essugar become the new faucet-pipeline-js?

PS: Out of curiosity, your second comment suggests you're using React's already-optimized distribution with exclude, but use NODE_ENV in your own source code as well?

I generally consider branching on NODE_ENV in first-party code a potential footgun, but I've had occasion to do so in the past (to conditionally enable/disable Redux devtools). That's not what was going on this time around, though: The NODE_ENV error came from React's CommonJS entrypoint, where they branch on the environment to select either the optimised or the dev build (relying on the bundler to tree-shake things correctly).

In particular, naming strikes me as a bit of a tricky issue, at least I don't have any good ideas on what that config option should look like.

I agree. Webpack uses "define", which has a distinctly C preprocessor-y vibe that probably isn't very intelligible, and Rollup uses "replace", which is much too generic (yet adequate, since – as I discovered – it really just does string replacement without considering the source file's syntax at all). Maybe the naming problem can be alleviated by reducing the scope?

If the goal is to only support process.env.NODE_ENV (and, perhaps, __DEV__), I don't see many ways in which faucet-pipeline-nodeenv (with an env or nodeEnv config key, as faucet-pipeline-essugar does) might be construed as ambiguous.

[ngrewe]

I've updated my aforementioned branch to have this functionality pluggable. The demonstrator that works with this (after npm link-ing) is available here.

[larsrh]

What is the relation to faucet-pipeline-js? Do you plan to eventually have faucet-pipeline-essugar become the new faucet-pipeline-js?

So far, faucet-pipeline-essugar has been a playground for a next-gen JS pipeline. Currently we don't have internal consensus yet about the future direction.