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

fix: ECMAScript Modules in Node.js #2130

Merged
merged 2 commits into from
May 19, 2023

Conversation

onemen
Copy link
Contributor

@onemen onemen commented May 7, 2023

Update all primitives to support ECMAScript Modules, resolve #1848.

  1. Add "exports" field to all all primitives package.json.
    typescript: ECMAScript Modules in Node
    node: exports
  2. Update "module" field file extensions to be ".mjs", this is also match parcel docs
  3. Make a copy of index.d.ts to index.d.mts, see the code in scripts/fix-type-defs-imports.js
    this is the relevant quote from typescript:
    It’s important to note that the CommonJS entrypoint and the ES module entrypoint each needs its own declaration file, even if the contents are the same between them

this is the relevant section from package.json

  "exports": {
    ".": {
      "import": {
        "types": "./dist/index.d.mts",
        "default": "./dist/index.mjs"
      },
      "require": {
        "types": "./dist/index.d.ts",
        "default": "./dist/index.js"
      }
    }
  },
  "source": "./src/index.ts",
  "main": "./dist/index.js",
  "module": "./dist/index.mjs",
  "types": "./dist/index.d.ts",

@onemen
Copy link
Contributor Author

onemen commented May 7, 2023

@benoitgrelard , @andy-hook

I hope you can review and merge it soon.

keep bringing us your grate UI library.

Copy link

@flex-haegul flex-haegul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@SoYoung210
Copy link

SoYoung210 commented May 11, 2023

@benoitgrelard , @andy-hook
There is currently an issue with ESM support when creating libraries using radix-ui
Hopefully this PR will be reviewed and fixed soon.

Copy link
Contributor

@benoitgrelard benoitgrelard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this @onemen, it all looks good I think.

I'll ping you back here once the RCs have been generated, it would be great if you could then test out locally using the RC to confirm that it all works as you expect with ESM. 🙏

@benoitgrelard benoitgrelard merged commit 4ed6ff3 into radix-ui:main May 19, 2023
@onemen
Copy link
Contributor Author

onemen commented May 19, 2023

OK, Thank you

@benoitgrelard
Copy link
Contributor

Ok, they have been generated, you can see all the versions here:

b61f185

Let me know if it all works as you expected locally when using ESM.

@onemen
Copy link
Contributor Author

onemen commented May 19, 2023

just tested

    "@radix-ui/react-checkbox": "^1.0.4-rc.6",
    "@radix-ui/react-dialog": "^1.0.4-rc.7",
    "@radix-ui/react-dropdown-menu": "^2.0.5-rc.11",
    "@radix-ui/react-popover": "^1.0.6-rc.11",
    "@radix-ui/react-separator": "^1.0.3-rc.6",
    "@radix-ui/react-tabs": "^1.0.4-rc.6",

on this project https://github.com/epicweb-dev/rocket-rental

everything work ok

we have to use a workaround and import from dist/index.js before this rc

- import Checkbox from '@radix-ui/react-checkbox/dist/index.js'
- import DropdownMenu from '@radix-ui/react-dropdown-menu/dist/index.js'
+ import * as Checkbox from '@radix-ui/react-checkbox'
+ import * as DropdownMenu from '@radix-ui/react-dropdown-menu'

@benoitgrelard
Copy link
Contributor

Great! Glad it all works correctly.

And thanks again for the contribution! 🙏

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.

Radix is not correctly resolved by modern Node.js ESM spec
4 participants