jkbrzt / rrule

JavaScript library for working with recurrence rules for calendar dates as defined in the iCalendar RFC and more.
https://jkbrzt.github.io/rrule
Other
3.27k stars 507 forks source link

Switch from faux-ESM to real ESM exports #548

Open karlhorky opened 2 years ago

karlhorky commented 2 years ago

Supersedes #512

Reporting an issue

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

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",
  ...
}

Does version 2.7.1 fix this? Short answer: No.

Longer answer (from https://github.com/jakubroztocil/rrule/issues/512#issuecomment-1180368129 ):

@davidgoli thanks for the new 2.7.1 version!

However, it does not resolve the issue - 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);

Demo (on Replit)

https://replit.com/@karlhorky/MotionlessUsedLoop#index.mjs

file:///home/runner/MotionlessUsedLoop/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:128:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:194:5)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:385:24)
repl process died unexpectedly: exit status 1
Screen Shot 2022-10-19 at 09 58 52
ryantan commented 1 year ago

Is this closed now? I can do import { RRule } from 'rrule'; with no issues

karlhorky commented 1 year ago

@ryantan are you using ES Modules with Node.js? Eg. with "type": "module" or an *.mjs file, no other transpilers / compilers like TypeScript or Babel?

If not (eg. if you're using TypeScript), then probably internally it is translating it to a version that works with your Node.js version.

I have added this Replit to the original post to demonstrate the problem:

https://replit.com/@karlhorky/MotionlessUsedLoop#index.mjs

file:///home/runner/MotionlessUsedLoop/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:128:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:194:5)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:385:24)
Screen Shot 2022-10-19 at 09 58 52
eytanProxi commented 1 year ago

Any news ?

splurgebudget commented 1 year ago

Same - still getting this with 2.7.2

caioamaral commented 1 year ago

I'm stuck here too

karlhorky commented 1 year ago

Workaround

Until this issue is properly resolved, here are some things that you may want to try:

My solution is working for my project, which is using tsc, tsx and vite:

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

If you're getting an error about the default export, another thing to try is the import * as pkg ... syntax:

import * as pkg from 'rrule';
const { RRule } = pkg;
splurgebudget commented 1 year ago

@karlhorky 's workaround didn't work for me. I was trying to reuse the same code in both Node and on the frontend (browser)... and either of the options would fail on one or the other.

Ultimately, I fixed it using dynamic import like so:

let rrule = await import("rrule");
let RRule = rrule.RRule;
if (!RRule) RRule = rrule.default.RRule;
karlhorky commented 1 year ago

Maybe your project is still CommonJS? You could try switching to ESM (ECMAScript Modules) by adding "type": "module" to your package.json (additional configuration may be needed).

codingedgar commented 1 year ago

My workarround for working with Vite on the browser and node 20 / testing was:

import * as pkg from 'rrule';
const { RRule } =  pkg.default || pkg;

In case anyone needs help.

mognutecnologia commented 1 year ago

I had the same scenario as @splurgebudget , where I am sharing code between frontend and backend through Npm Workspaces Monorepo. Solved it as @codingedgar , but tweaking a bit to pass through Typescript compilation, in case it helps somebody:

import * as pkg from "rrule";
const tsIgnorePkg = pkg as any;
const { RRule } = tsIgnorePkg.default || tsIgnorePkg;
septatrix commented 11 months ago

For me the workarounds do not work with Nuxt.js and I either get the above error about not being able to import rrule without it or the tslib module not being found when trying the workaround (though it would even be installed - not sure however why it would need it) :/

PS: I had rrule imported in several files split across my codebase (and third party packages) so my only option was to use the build.transpile option of nuxt. However, I also needed to add tslib to the list of packages which have to be transpiled for everything to function properly. That seems to have fixed it for me.

michaelhays commented 5 months ago

Any chance we can get this implemented? The faux-ESM of this package has caused me quite a few headaches over the past year.

I'm happy to create a PR if I could have some assurance that this is a priority.

JohnRPB commented 3 months ago

I also have issues with ESM on the one hand, or webpack bundling on the other, and the fix that works for me is re-exporting from a wrapper file like this:

import * as rrule from 'rrule';

const getDefault = pkg => pkg.default; 

export const RRule = rrule.RRule || getDefault(rrule)?.RRule;

By using a closure, we avoid webpack's "no default export" error.

septatrix commented 3 months ago

Even though I am generally not a huge fan of forking unless the maintainer has officially abandoned/archived a project it might be time to do this here. The last commit is 6 months old, the last release was 2 years ago, and there are a bunch of unresolved PRs.

I would be happy if @jkbrzt could give a statement on whether or not he would still like to maintain and keep this project alive? Maybe also @davidgoli who seems to have contributed a decent amount.

davidgoli commented 3 months ago

I don't have time to maintain this project any more, and I don't have a professional use for it at the moment either. I would be happy to hand the reins to someone who is more committed than I am.

macmillen commented 2 months ago

+1 I need this as well

gajus commented 2 months ago

If you don't mind patching your dependencies, here is a workaround:

diff --git a/CHANGELOG.md b/CHANGELOG.md
deleted file mode 100644
index bfaae2edc85d2ac819c975cc8f72a98678a6ab21..0000000000000000000000000000000000000000
diff --git a/dist/es5/rrule.js b/dist/es5/rrule.js
index b671aaad592beda14cd0a778fe25b412a0effd89..9684910e7e25f99e9ae7b2df8d3121ac12b5530f 100644
--- a/dist/es5/rrule.js
+++ b/dist/es5/rrule.js
@@ -1,7 +1,11 @@
 (function webpackUniversalModuleDefinition(root, factory) {
-   if(typeof exports === 'object' && typeof module === 'object')
-       module.exports = factory();
-   else if(typeof define === 'function' && define.amd)
+   if(typeof exports === 'object' && typeof module === 'object') {
+        const rrule = factory();
+
+        module.exports.rrulestr = rrule.rrulestr;
+        module.exports.datetime = rrule.datetime;
+        module.exports.RRule = rrule.RRule;
+    } else if(typeof define === 'function' && define.amd)
        define([], factory);
    else if(typeof exports === 'object')
        exports["rrule"] = factory();
diff --git a/package.json b/package.json
index 4035e6fcefc690b033f8eb20c557f11980bb8422..4e224e1ce2f64aeab3b863805714853a98cb29e8 100644
--- a/package.json
+++ b/package.json
@@ -13,7 +13,6 @@
   ],
   "author": "Jakub Roztocil, Lars Schöning, and David Golightly",
   "main": "dist/es5/rrule.js",
-  "module": "dist/esm/index.js",
   "types": "dist/esm/index.d.ts",
   "repository": {
     "type": "git",