Module format in lib/module is using cjs not esm

[natew]

Description

This is causing vite to blow up, and it's really hard to fix without patching the package. I can help contribute a fix just want to get confirmation this is ok to change.

Steps to reproduce

1. Import react-native-screens from vite
2. It errors, even when using transformMixedEsModules
3. git clone git@github.com:tamagui/tamagui.git
4. yarn
5. yarn tamastack
6. Load iOS simulator and load app, it errors

Snack or a link to a repository

https://github.com/tamagui/tamagui

Screens version

3.22.1

React Native version

0.72.0

Platforms

iOS

JavaScript runtime

None

Workflow

None

Architecture

None

Build type

None

Device

None

Device model

No response

Acknowledgements

Yes

[github-actions]

Hey! 👋

The issue doesn't seem to contain a minimal reproduction.

Could you provide a snack or a link to a GitHub repository under your username that reproduces the problem?

[natew]

It doesn’t have a minimal repro, but it’s easy to see the commonjs and esm export are mixed inside lib/modules/index.native.js

[tboba]

Hi @natew, thanks for reporting this issue!
We're aware of mixing CJS and ESM types in our codebase (and there's also a PR that suggests removing type exports in #1830 which I believe might be related to your issue) but for now we're worried we might lose autocompletion and completely break linter if we'd decide to stick with just one format (ESM/CJS).

If you feel strong enough and you want to contribute a fix for that - please go ahead! We've already got this in a roadmap, but any help is appreciated 🙏

[jkhaui]

This is an issue I personally feel strongly about. Unfortunately I probably can't contribute a fix as I'm just a lowly web dev (I'm building on react-native-web, react-navigation, react-freeze, etc. using Vite as the bundler, hence why I'm here), but hopefully I can provide a helpful resource and some context as to why IMO it's important for the ecosystem of RN libraries to prioritise fixing these CJS/ESM issues.

Firstly, I came across this tool: it appears to be a great reference for all the things that probably need to be updated in order to achieve semantically correct CJS/ESM exports: https://publint.dev/react-native-screens@3.27.0

Regarding the issue Nate raised here, I think he's primarily referring to the use of CJS require import statements instead of something like await import(...). This can be enough to completely break a Vite-based toolchain especially if the package is used as a transitive dep (Vite has in-built heuristics where it tries to fix deps with mixed syntax, but its ability is pretty limited).

The first image below shows some imports from react-native-screen's lib/module/index.native.js ESM module build:

The second screenshot below now shows the CommonJS build; as we can see, this block of imports is identical to the ESM module file. As I understand it, this is problematic because ESM generally doesn't support dynamic imports like they're being used here (this would break static analysis tools—treeshaking bundlers can't accurately detect which files should be included in the output of the final application code consuming react-native-screens):

Now, something I've noticed is many of the major RN libraries (i.e. this definitely isn't specific to react-native-screens) don't appear to adhere strictly to CJS/ESM import/export conventions, despite distributing both CJS & ESM builds (created by react-native-builder-bob).

I think this poses a long-term issue with modern JS looking to be trending towards isomorphic development (e.g. React Server Components). And the main value proposition of ESM is arguably to provide a unified standard adopted by both server and client, and Vite is an ESM-first bundler which is becoming the de facto choice for most new JS projects today.

So, why should RN libraries care? Because there may now be an upcoming generation of developers who want to use RN in different environments (e.g. https://stereobooster.com/posts/react-native-web-with-vite).

However, its ecosystem is so tightly coupled to only working with Metro & Babel. Thus using other bundlers is so difficult it may as well be impossible. This problem is compounded by RN's main libs essential for scaffolding basic functionality (e.g. navigation, views) being heavily dependent on one another. If a dev can't get one of these dependencies working in an app, then it's likely none of the others can be integrated either.

In summary: if my reasoning above is correct, then this is a big shame... because there's so much potential innovation that will never be realised. 😢

[tboba]

Hi @natew @jkhaui! I've managed to remove CJS files while bundling the project, resulting in building only ESM files. I believe this should fix this issue, but I need your assistance 🕵️
Could you check if your project is building correctly while using changes from #1982?
You can easily check this by changing the version of react-native-screens to:

"react-native-screens": "software-mansion/react-native-screens#@tboba/remove-mixed-cjs"

Let me know what do you think about it.
Cheers!

[tboba]

Hi @natew @jkhaui, right now I'm trying to create an example with react-native-web and vite (to test changes from PR #1982) but unfortunately I'm stuck with couple of errors. Because of how react-native-screens is made, Vite is complaining about AppContainer.js file:

[vite] Internal server error: Failed to parse source for import analysis because the content contains invalid JS syntax. If you are using JSX, make sure to name the file with the .jsx or .tsx extension.
  Plugin: vite:import-analysis
  File: Project/node_modules/react-native/Libraries/ReactNative/AppContainer.js?v=7433b0e7:138:14
  136|          }}>
  137|          {this.props.children}
  138|        </View>
     |               ^
  139|      );
  140|  

As you can see, this error refers to the React Native internals, so probably changing this file manually from .js to .jsx could be a bad idea 🤔
I just wanted to ask if you stumbled upon the same issue before, while trying to run the project with react-native-screens? Do you have any example with correctly configured RN-web, react-native-screens and Vite that shows only the error mentioned in this issue?

My vite.config.js configuration:

import { defineConfig } from "vite";
import react from "@vitejs/plugin-react";

// https://tamagui.dev/docs/intro/installation
const extensions = [
  ".web.tsx",
  ".tsx",
  ".web.ts",
  ".ts",
  ".web.jsx",
  ".jsx",
  ".web.js",
  ".js",
  ".css",
  ".json",
  ".mjs",
];

const development = process.env.NODE_ENV === "development";

// https://vitejs.dev/config/
export default defineConfig({
  clearScreen: true,
  plugins: [react()],
  define: {
    // https://github.com/bevacqua/dragula/issues/602#issuecomment-1296313369
    global: "window",
    __DEV__: JSON.stringify(development),
    // https://tamagui.dev/docs/intro/installation
    DEV: JSON.stringify(development),
    "process.env.NODE_ENV": JSON.stringify(process.env.NODE_ENV),
  },
  resolve: {
    extensions: extensions,
    alias: [
      {
        find: /^react-native$/,
        replacement: "react-native-web",
      }
    ]
  },
  optimizeDeps: {
    exclude: ["react-native"],
    esbuildOptions: {
      resolveExtensions: extensions,
      // https://github.com/vitejs/vite-plugin-react/issues/192#issuecomment-1627384670
      jsx: "automatic",
      // need either this or the plugin below
      loader: { ".js": "jsx" },
      // plugins: [
      //   esbuildFlowPlugin(/\.(flow|jsx?)$/, (path) =>
      //     /\.jsx$/.test(path) ? "jsx" : "jsx"
      //   ),
      // ],
    },
  },
});

[tboba]

Small update from me: I've managed to run the example app with two screens in a navigator, but with use of @react-navigation/native-stack (and not react-native-screens/native-stack, as I tried before). However, I'm still not receiving errors about the mixed codebase 😅 Just wanted to ask here (so I could reproduce these errors), what is your configuration? Does this error happen only when Tamagui is bundled to the project? (I've also tried to use Tamagui, but I'm getting weird errors while I'm trying to configure it with TamaguiProvider 🤔)

[tboba]

Hi @natew, @jkhaui, since the linked PR has been merged, the mixed CJS/ESM case should be over - on Vite there shouldn't be any errors for now, but if you spot any - please let me know!

Cheers!