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

Add cjs support as conditional exports #183

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pravi
Copy link

@pravi pravi commented May 12, 2022

This adds support for both cjs and esm versions (Closes #176)

@chase-manning
Copy link
Collaborator

@pravi This approach will double the package size right?

@pravi
Copy link
Author

pravi commented May 16, 2022

@chase-manning yes, but it will allow people still using commonjs to continue using dateformat. Better than being stuck at old version.

@pravi pravi changed the title Add cjs cupport as conditional exports Add cjs support as conditional exports May 16, 2022
@pravi
Copy link
Author

pravi commented May 16, 2022

Also people using bundlers like webpack won't see a size increase in final bundle.

@matushorvath
Copy link

This would be really helpful. Currently we are stuck with using an old version because there are still tools that don't play nice with modules (I'm looking at you, jest and typescript). The support is getting better every month, so hopefully cjs support can be removed after some time, but I'd argue we are not there yet. The ~9k of increased package size is real, but I would still consider it a good trade-off. Thank you!

@guimard
Copy link

guimard commented May 19, 2022

This patch doesn't produce the wanted behavior: require('dateformat') returns an object, not a function

@pravi
Copy link
Author

pravi commented May 19, 2022

As @guimard mentioned, this will need a change in API, for example, grunt needs this patch

--- a/lib/grunt/template.js
+++ b/lib/grunt/template.js
@@ -6,7 +6,7 @@
 var template = module.exports = {};
 
 // External libs.
-template.date = require('dateformat');
+template.date = require('dateformat').default;
 
 // Format today's date.
 template.today = function(format) {

Any ideas to get the old behavious using just babel? @babel/plugin-proposal-export-default-from plugin and babel-plugin-add-module-exports don't change anything.

@pravi
Copy link
Author

pravi commented May 19, 2022

@chase-manning
Copy link
Collaborator

@pravi Could we please make some changes to this PR?

Can we please update the Readme to include reference to both import options.

Also, can we please add tests for importing and using the package using the cjs version.

@pravi
Copy link
Author

pravi commented May 20, 2022

@chase-manning please check if the changes are fine. I have converted one test to CJS, if you think this approach is fine. I can copy the remaining tests too. Or we can add a test_helper.esm and test_helper.cjs files which will handle the ESM and CJS parts and may be have some logic to detect cjs or esm in the tests. But I'm not sure how to do that.

@loganmzz
Copy link

loganmzz commented Oct 4, 2022

Where is it five monthes later ?

@Aravin
Copy link

Aravin commented Oct 23, 2022

Any plan to merge?

@Aravin
Copy link

Aravin commented Oct 23, 2022

Error

[2022-10-23T18:48:13.701Z] Stack: Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /Users/xxxxx/node_modules/dateformat/lib/dateformat.js

@droplet-js
Copy link

any news update?

@ffleandro
Copy link

Any updates on this?
I usually compile my trypescript code into cjs to use in AWS Lambda.

@BaderBC
Copy link

BaderBC commented Dec 22, 2023

@chase-manning
Any updates?

@huangganggui
Copy link

Any updates?

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.

dateformat not working with "require" ?
10 participants