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

Switch from faux-ESM to real ESM exports #512

Closed
6 of 7 tasks
karlhorky opened this issue Jun 7, 2022 · 8 comments
Closed
6 of 7 tasks

Switch from faux-ESM to real ESM exports #512

karlhorky opened this issue Jun 7, 2022 · 8 comments

Comments

@karlhorky
Copy link

karlhorky commented Jun 7, 2022

Reporting an issue

Thank you for taking an interest in rrule! Please include the following in
your report:

  • Verify that you've looked through existing issues for duplicates before
    creating a new one
  • Code sample reproducing the issue. Be sure to include all input values you
    are using such as the exact RRule string and dates.
  • Expected output
  • Actual output
  • The version of rrule you are using 2.7.0
  • Your operating system macOS Monterey 12.4
  • Your local timezone (run $ date from the command line
    of the machine showing the bug)

The "module" field in package.json is non-official, and not a real ESM specification of npm: https://stackoverflow.com/a/42817320/1268612

Switching away from "module" to use the "exports" and "type": "module" fields in the package.json would allow for better compatibility with ESM, everywhere it is used.

You may even consider making it a pure ESM package, as mentioned here: https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c

Expected output (see "type" and "exports" fields):

$ cat node_modules/rrule/package.json
{
  "name": "rrule",
  "version": "2.7.0",
  ...
  "main": "dist/es5/rrule.js",
  "type": "module",
  "exports": "./dist/esm/src/index.js",
  "types": "dist/esm/src/index.d.ts",
  ...
}

Actual output (see the outdated faux-ESM "module" field):

$ cat node_modules/rrule/package.json
{
  "name": "rrule",
  "version": "2.7.0",
  ...
  "main": "dist/es5/rrule.js",
  "module": "dist/esm/src/index.js",
  "types": "dist/esm/src/index.d.ts",
  ...
}
@karlhorky karlhorky mentioned this issue Jun 7, 2022
7 tasks
@davidgoli
Copy link
Collaborator

Thanks for the suggestion. I don't think we're ready to abandon cjs users at this point, especially considering how new ESM is to Node itself, but I would like to improve the ESM support. What do you think about a solution like the one outlined here: https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1

@karlhorky
Copy link
Author

Maybe it would work, I've heard some (rare) accounts of projects providing both ESM and CJS successfully. Any kind of improvement would be great! 👍

@davidgoli
Copy link
Collaborator

Please take a look at the latest (2.7.1) and let me know how it's working for you now

@karlhorky
Copy link
Author

karlhorky commented Jul 11, 2022

@davidgoli thanks for the new version!

rrule is still not published as ESM.

Importing and using rrule in the following file (import copied from the rrule readme) leads to an error.

index.mjs

import { RRule } from 'rrule';

console.log(RRule);

The error is this below:

$ node index.mjs
file:///Users/k/p/ical-move-events/index.mjs:1
import { RRule } from 'rrule';
         ^^^^^
SyntaxError: Named export 'RRule' not found. The requested module 'rrule' 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 'rrule';
const { RRule } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:124:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:179:5)
    at async Loader.import (node:internal/modules/esm/loader:178:24)
    at async Object.loadESM (node:internal/process/esm_loader:68:5)
    at async handleMainPromise (node:internal/modules/run_main:63:12)

Luckily there is a workaround and it is printed out in the error message, but a lot of users will probably not notice it or not understand:

import pkg from 'rrule';
const { RRule } = pkg;

console.log(RRule);

So probably this issue should be reopened, for seamless real ESM support.

@karlhorky
Copy link
Author

@davidgoli do you think you could reopen this issue? I think it is not resolved yet.

@karlhorky
Copy link
Author

karlhorky commented Aug 16, 2022

@davidgoli I've opened a new issue for this instead:

Superseded by #548

@JuicyZest
Copy link

Same Issue here,
import pkg from 'rrule';
const { RRule } = pkg;
This workaround resolves the module error, but gives me a RRule is not a constructor error when trying to create a new RRule object.
Anyone ever found a solution ?

@karlhorky
Copy link
Author

@JuicyZest this issue is closed, can you copy your comment above to the current issue over here?

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

No branches or pull requests

3 participants