felixge / node-dateformat

A node.js package for Steven Levithan's excellent dateFormat() function.
MIT License
1.3k stars 156 forks source link

Consider dual CJS/ESM export #169

Open mcollina opened 3 years ago

mcollina commented 3 years ago

ESM is not ready for prime time in a significant part of the Node.js usage needs the stabilization of Loaders, which are still experimental and prone to changes. Therefore I'm not ready switching my modules to be ESM-only.

Would you consider dual publishing for this module?

chase-manning commented 3 years ago

Oh great idea! Would be keen to do this 😄 I don't know how to though ha ha, if it's easy could you raise a PR? Otherwise I'll do some investigating and see if I can work it out 😄

mcollina commented 3 years ago

@kibertoad maybe could you help?

kibertoad commented 3 years ago

@chase-manning Does https://github.com/ai/dual-publish/ look like a suitable solution for you?

chase-manning commented 3 years ago

Oh yes thanks that looks like a good solution!

mcollina commented 3 years ago

Note that it would change

import dateFormat from "dateformat";

to

import { dateFormat } from "dateformat";

What's the problem with webpack and default exports @kibertoad ?

kibertoad commented 3 years ago

@mcollina Problem is this: https://github.com/webpack/webpack/issues/6584 From my understanding it is mostly addressed in Webpack 5: https://github.com/webpack/webpack/issues/7973#issuecomment-683951296

mcollina commented 3 years ago

Ok thanks

chaeron commented 3 years ago

The problem with this approach is that existing code that uses dataformat and CJS requires will have to change to use:

import { dateFormat } from "dateformat";

instead of

import dateFormat from "dateformat";

That is a huge headache for those of us that use dateformat extensively.

I would prefer to see two separate npm packages:

brookmg commented 3 years ago

If anyone else is having a similar problem, check out https://github.com/knowledgecode/date-and-time. it supports common js but has some formatting different from this module. I am not in any way affiliated with the project only sharing it because it solved my problem.

kibertoad commented 3 years ago

The problem with this approach is that existing code that uses dataformat and CJS requires will have to change

If released as a semver major, how is that an issue?

chase-manning commented 3 years ago

Hey @jimmywarting what's your take on this issue? 😄

jimmywarting commented 3 years ago

I say go for ESM only. @sindresorhus is converting all of his 1000+ packages, node-fetch and submodules use esm-only, LinusU is switching to ESM-only as well whenever he touch a own commonjs project of his. WebTorrent and all sub-modules is also in the process of being converted to ESM-only. And from the looks of it Babel 8 is also going to become esm only There is no doubt that esm is the feature and more and more is converting to ESM to support both Deno and Browser imports without a bundler or npm package. Every project i own is also converted to esm only

I think maintaining a dual package support is a #developer-pain. it duplicates all the files and package size. it introduces bundlers/compilers (and this dose not apply to node-dateformat but compiling everything into one cjs file means you would have instanceof problems with other dependencies if multiple packages uses the same dependencies)

I think those who are stuck with cjs can use the previous major version and pin it to v4. That's what a semantic Major breaking change means... Don't update it if you are not ready for it.

while reading https://github.com/pinojs/pino-pretty/pull/235 i would hate for packages to be divided up by diffrent forks (cjs vs esm) where there are no longer one single source of truth. it would be grate if the main upstream repo got all the updates.

There are still ways after all to load esm only packages using the async import() available from cjs after all.

ESM have been out for a long time and it's marked as stable https://nodejs.org/api/esm.html @mcollina you talk about Loaders that are experimental, do you need something that can "resolve" stuff?

I really think NodeJS should do something about this split war between cjs and esm


But i don't think i have a much saying in this as i'm just a user that have contributed to the project. I don't consider myself a Collaborator or a member of this project. If you want to support dual packages, then go for it, it's up to you.

jsumners commented 3 years ago

I think those who are stuck with cjs can use the previous major version and pin it to v4. That's what a semantic Major breaking change means... Don't update it if you are not ready for it.

Nonsense. That means no security fixes because it adds more complicated developer maintenance that isn't going to be done. This is the response of people who do not maintain a large production ecosystem.

There are still ways after all to load esm only packages using the async import() available from cjs after all.

None that don't require users to refactor their code into a big pile of promises.

  • support top level await in commonjs

Impossible.

Likely also impossible due to ESM spec.

  • start discouraging cjs from new projects and marking cjs as legacy, the more ppl that create cjs project the more problem they will have to update to esm in the feature

Again, this is the response of people who do not maintain large production ecosystems. Even if everything else would be viable (e.g. the budget to convert everything to a completely different paradigm were actually possible for anyone), the fact that ESM is not able to be instrumented means actual business cannot "just make the switch."

Yes, semver major implies breaking changes. But we will disagree all day long that it includes completely changing the module contract. That's the realm of a new module with a new name. Don't care who is doing it.

chaeron commented 3 years ago

I'm with @jsumners on this one....well said James!

The whole CommonJS vs ESM module thing is a huge stinking mess and the horns of a serious dilemma for those of us that have a large production code base to maintain and extend.

itsHusky commented 3 years ago

I am not too sure if this is addressed in the right topic, but I wasn't able to use version 5.0.1, as the module.exports statement is missing in that version. So, I went back to version 4.6.3, which worked for me. I just wondered, as the ReadMe still says: Added a module.exports = dateFormat; statement at the bottom. Version 5.0.1 does not seem to have the module.exports statement anymore.

import dateFormat from "dateformat"; gave me the following error :

ParseError: 'import' and 'export' may appear only with 'sourceType: module'

I use TypeScript and bundle modules with browserify.

jsumners commented 3 years ago

I am not too sure if this is addressed in the right topic, but I wasn't able to use version 5.0.1, as the module.exports statement is missing in that version. So, I went back to version 4.6.3, which worked for me. I just wondered, as the ReadMe still says: Added a module.exports = dateFormat; statement at the bottom. Version 5.0.1 does not seem to have the module.exports statement anymore.

import dateFormat from "dateformat"; gave me the following error :

ParseError: 'import' and 'export' may appear only with 'sourceType: module'

I use TypeScript and bundle modules with browserify.

No. This seems completely off topic and should be a new issue.

BasilaryGroup commented 2 years ago

If anyone else is having a similar problem, check out https://github.com/knowledgecode/date-and-time. it supports common js but has some formatting different from this module. I am not in any way affiliated with the project only sharing it because it solved my problem.

Thanks. We just ported our nodejs apps to https://github.com/knowledgecode/date-and-time. It was simple to do.