Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] Fix entrypoints for CJS and ESM modules #4792

Merged
merged 1 commit into from
Sep 24, 2023
Merged

[core] Fix entrypoints for CJS and ESM modules #4792

merged 1 commit into from
Sep 24, 2023

Conversation

jopesh
Copy link
Contributor

@jopesh jopesh commented Sep 19, 2023

I have been experiencing issues with the v2 release of Remix and Mantine v7 (see https://discord.com/channels/854810300876062770/1066443903080869889/1153294961324851220). Since Remix v2 is now ESM-only by default, it apparently changed the interaction with the @mantine/* packages:

import { ColorSchemeScript, MantineProvider } from "@mantine/core";
         ^^^^^^^^^^^^^^^^^
SyntaxError: Named export 'ColorSchemeScript' not found. The requested module '@mantine/core' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from '@mantine/core';
const { ColorSchemeScript, MantineProvider } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:124:21)
    at ModuleJob.run (node:internal/modules/esm/module_job:190:5)

As far as I can tell and by testing the issue with a local modified mantine build, it seems that Remix imports the Mantine packages as CommonJS – even though there are ESM files provided.

Jacob Parris from the Remix community pointed me to this test: https://arethetypeswrong.github.io/?p=%40mantine%2Fcore%407.0.0

It led me to this problem description: https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FallbackCondition.md in combination with this one https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseCJS.md

Digging deeper and testing it on my machine, the problem projected on Mantine appears as follows: since Mantine is no ESM-native package (missing type: "module" in package.json), some bundlers (or just native Node.js?) treat it as a CommonJS package, hence the index.d.ts type declarations defined early in the package.json. To solve this, one would have to declare different type AND module entrypoints for both ESM and CJS to separate them consequently – like in this example :

{
  "name": "pkg",
  "exports": {
    ".": {
      "import": {
        "types": "./lib/index.d.mts",
        "default": "./esm/index.mjs"
      },
      "require": {
        "types": "./lib/index.d.ts",
        "default": "./cjs/index.js"
      }
    }
  }
}

After changing the ESM output to .mjs, creating a duplicate index.d.mts file and updating the package.jsonas shown above, the packages work with Remix v2 (tested @mantine/core and /hooks)

It also still works with the remix-min-template using >v1 of Remix.

Disclaimer: I am not expert in package distribution and this whole CJS/ESM rabbit hole. Hence, I do not declare final wisdom here – just some uneducated afternoon troubleshooting.

@chrisbendel
Copy link
Contributor

hey @jopesh, I came here looking around after getting the following error - this looks to be in the same realm:

/Users/cb105/code/research-chatbot/node_modules/@mantine/core/esm/index.js:2
export { RemoveScroll } from 'react-remove-scroll';
^^^^^^

SyntaxError: Unexpected token 'export'
    at internalCompileFunction (node:internal/vm:74:18)
    at wrapSafe (node:internal/modules/cjs/loader:1141:20)
    at Module._compile (node:internal/modules/cjs/loader:1182:27)
    at Module._extensions..js (node:internal/modules/cjs/loader:1272:10)
    at Object.require.extensions.<computed> [as .js] (/Users/cb105/code/research-chatbot/node_modules/ts-node/src/index.ts:1608:43)
    at Module.load (node:internal/modules/cjs/loader:1081:32)
    at Function.Module._load (node:internal/modules/cjs/loader:922:12)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:169:29)
    at ModuleJob.run (node:internal/modules/esm/module_job:194:25)

Following this in case this might be a fix. Thanks for this work!

@nshelia
Copy link

nshelia commented Sep 23, 2023

Same issue with vite-plugin-ssr. Cannot upgrade Mantine until this is fixed.

@cyco130
Copy link

cyco130 commented Sep 24, 2023

Same issue with Rakkas.

Node.js (without a bundler) cannot import ESM files with a .js extension if they're under a package.json that doesn't include "type": "module". Vite (and apparently Remix) don't bundle server dependencies by default so they both fail.

The fix would be to rename ESM files as .mjs (or add "type": "module" to package.json and rename CJS files as .cjs).

Here's the most minimal reproduction possible. Just importing @mantine/core from an index.mjs file run with node index.mjs fails with Node emitting a warning that summarizes the issue:

Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.

@rtivital rtivital merged commit db6f0e2 into mantinedev:master Sep 24, 2023
1 check passed
@rtivital
Copy link
Member

Thanks!

@rtivital
Copy link
Member

I've published 7.0.1-alpha.0 with these changes, I'll validate whether they work in every framework and report here. You are also welcome to check whether the changes fixed the issue.

@ssch-99
Copy link

ssch-99 commented Sep 24, 2023

With Remix I get still the same error (Made the project according to instructions: https://mantine.dev/guides/remix/)
Screenshot 2023-09-24 at 18 08 46

package.json:
Screenshot 2023-09-24 at 18 09 45

@rtivital
Copy link
Member

You need to rename postcss.config.js to postcss.config.cjs

@rtivital
Copy link
Member

rtivital commented Sep 24, 2023

I've created a PR for remix v2 migration, it works fine, although most of the things like storybook and jest are not supported, but there is not much I can do on my side.

mantinedev/remix-template#11

@wscotten
Copy link

wscotten commented Oct 10, 2023

I am still getting this error in nextjs v13.4.7
Mantine v7.1.2

@rtivital
Copy link
Member

Try updating to the latest version of Next.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants