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

Not working with ESM on Node 10 #252

Closed
damianobarbati opened this issue May 2, 2018 · 18 comments · Fixed by #1218
Closed

Not working with ESM on Node 10 #252

damianobarbati opened this issue May 2, 2018 · 18 comments · Fixed by #1218

Comments

@damianobarbati
Copy link

Hey, any chance to have luxon work with Node ESM modules?

import { DateTime } from 'luxon';
           ^^^^^^^^
SyntaxError: The requested module 'luxon' does not provide an export named 'DateTime'
at ModuleJob._instantiate (internal/modules/esm/module_job.js:89:21)
@icambron
Copy link
Member

icambron commented May 3, 2018

I've never used ESM, so I am not sure. Pretty hard to imagine why it thinks that. E.g. see the end of http://moment.github.io/luxon/node/luxon.js

I'd take a fix to the toolchain that helped here, but I have no idea how to help myself

@ericmorand
Copy link

ericmorand commented May 3, 2018

Aren't ESM modules an experimental feature of node 10?

@damianobarbati
Copy link
Author

Yes, and soon moving to harmony flag. Let's be prepared! :)

NB: I've been using them for weeks by now and they work seamlessly (only pain in the ass is you can't import .json as you used to require .json ...).

@ericmorand
Copy link

(only pain in the ass is you can't import .json as you used to require .json ...)

That's actually a very good thing if you ask me. JSON files are not modules in any way and this was leading to a lot of potential problems when you were requiring modules in the blind.

Anyway, yes, ESM is just around the corner. Why is it not working? Isn't doing this enough to support ESM?

export { DateTime, Duration, Interval, Info, Zone, FixedOffsetZone, IANAZone, LocalZone, Settings };

export { DateTime, Duration, Interval, Info, Zone, FixedOffsetZone, IANAZone, LocalZone, Settings };

@damianobarbati
Copy link
Author

damianobarbati commented May 3, 2018

@ericmorand definitely: but main entry in package.json is not linking src/luxon.js (source) but build/node (transpiled).
Here:

"main": "build/node/luxon.js",

@ericmorand
Copy link

Oh yes, good one.

@icambron
Copy link
Member

icambron commented May 3, 2018

Yes, we link to the built one, where the relevant code is:

exports.DateTime = DateTime;
exports.Duration = Duration;
exports.Interval = Interval;
exports.Info = Info;
exports.Zone = Zone;
exports.FixedOffsetZone = FixedOffsetZone;
exports.IANAZone = IANAZone;
exports.LocalZone = LocalZone;
exports.Settings = Settings;

@damianobarbati
Copy link
Author

Maybe .mjs extension is required?

@icambron
Copy link
Member

Well, even if that's true, I can't just export that or it will break versions of Node that don't support it. What we need is something like:

  1. Make the build produce a separate file
  2. Point to it with a different key in package.json that Node will know to import from

No idea if 2 is possible, but I'm unclear on what the path forward is supposed to be otherwise. How are libraries expected to provide these modules?

@pho3nixf1re
Copy link

pho3nixf1re commented Jun 19, 2018

You can set the main to index and provide both an index.js and index.mjs. Then ESM will pick up the mjs one and the rest of us will use js.

@pho3nixf1re
Copy link

You might also consider using https://github.com/standard-things/esm to remove the need for duplication and some pre-publish build steps.

@icambron
Copy link
Member

Ah, that's the kind of answer I was looking for. I'll get to this, but if someone wants to give it a shot, that would be great

@damianobarbati
Copy link
Author

You can set the main to index and provide both an index.js and index.mjs. Then ESM will pick up the mjs one and the rest of us will use js.

@pho3nixf1re is that a default behaviour when esm is enabled?
If I set "main": "index" without extensions thus it is going to look for index.mjs first, index.js second?
Any reference on it?

@pho3nixf1re
Copy link

There is nothing formal about it. If you use native node imports then mjs is the only thing that will resolve. If you use CommonJS then js will be resolved unless you have some configuration that changes that to force mjs. The esm module will resolve mjs before js if CommonJS support is enabled. I saw this described by the esm maintainer in an issue some time ago but I don't remember where. I use the technique for all of my Node packages. When building for the web esm can't be used of course. (It's a node hook).

@jdalton
Copy link

jdalton commented Jul 10, 2018

Hi 👋!

Node core, Node Module WG member, and creator of esm here. I just wanted to pop in and say

Yes, and soon moving to harmony flag.

There is no set time for Node to move from --experimental-modules to an unflagged implementation. There is also no guarantee on what the final implementation will look like. While the .mjs extension will be used to disambiguate. The extension may not be-the-only-way to disambiguate. There are several ongoing discussions in the working group about what ESM support will look like in Node. Many possible outcomes are incompatible with the current WIP implementation.

The esm loader lets you write ESM today and migrate to whatever the ESM of tomorrow will be. So you can use esm and ship ES modules that are consumable by CJS and compatible with code coverage, bundlers, APM, unit test, and mocking libraries. As Node's support story solidifies you can tweak, and migrate gradually over.

@L-as
Copy link

L-as commented Jan 9, 2020

FWIW, you can set "type": "module" in package.json, import src/luxon.js directly, and it will work fine, though it breaks packages that depend on luxon that don't use modules.

L-as added a commit to L-as/luxon that referenced this issue Jan 9, 2020
Partial fix for not being able to do e.g. `import {DateTime} from "luxon"`.
This *still* won't work, but `import {DateTime} from "my/own/luxon/src/luxon.js" will.

See moment#252
@AGBrown
Copy link

AGBrown commented Jul 19, 2020

I'm on a later version of node, but I found that I had really strange intermittent issues with luxon, import and typescript. In some files the import was fine, in others it was not. The error was only ever at runtime (in node) and was along the lines of

import { Settings, Interval } from 'luxon';
                   ^^^^^^^^
SyntaxError: The requested module 'luxon' does not provide an export named 'Interval'
    at ModuleJob._instantiate (internal/modules/esm/module_job.js:92:21)
    at async ModuleJob.run (internal/modules/esm/module_job.js:107:20)
    at async Loader.import (internal/modules/esm/loader.js:179:24)

or

import { DateTime } from 'luxon';
         ^^^^^^^^
SyntaxError: The requested module 'luxon' does not provide an export named 'DateTime'

or (this time DateTime is fine, but Info is not)

import { DateTime, Info } from 'luxon';
                   ^^^^
SyntaxError: The requested module 'luxon' does not provide an export named 'Info'

I solved it in one file with:

import luxon from 'luxon';

const { DateTime, Settings } = luxon;

and yet in another file in the same project I have this, with no issues:

import { DateTime, Info } from 'luxon';

Versions are:

$ npm -v && node -v && npm list --depth=0 | grep luxon
6.14.5
v12.17.0
+-- @types/luxon@1.24.1
+-- luxon@1.24.1

relevant package.json and tsconfig.json attached (as text files as git doesn't allow json files to be attached)

package.json.txt
tsconfig.json.txt

@Darkle
Copy link

Darkle commented Sep 18, 2021

Any movement on this? I am getting the same errors. I tried @AGBrown suggestion but it didnt work for me.
Im on luxon version 2.0.2

@pho3nixf1re's suggestion seems like the simplest fix for luxon. Or alternatively you cold rename all the /src .js files to .mjs

Docs https://nodejs.org/dist/latest-v14.x/docs/api/packages.html#packages_package_json_and_file_extensions

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

Successfully merging a pull request may close this issue.

8 participants