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

fix: add module.exports CJS syntax, add example with require #87

Conversation

aeworxet
Copy link
Collaborator

@aeworxet aeworxet commented Oct 24, 2022

This PR:

  • adds module.exports CJS syntax, to maintain backward compatibility with Node.js projects that use CJS module system,
  • adds two more examples of code usage written in JavaScript, in addition to the one already provided written in TypeScript.

Fixes #81

@aeworxet
Copy link
Collaborator Author

@magicmatatjahu

@derberg derberg changed the title fix: change 'bundle' export to CJS syntax and add example with 'require' (#81) fix: change bundle export to CJS syntax and add example with require Oct 25, 2022
@derberg
Copy link
Member

derberg commented Oct 25, 2022

@aeworxet you need to fix tests

@derberg
Copy link
Member

derberg commented Oct 25, 2022

I just tested with my local project and looks like you can do both:
export default async function bundle and module.exports = bundle;
after transpilation it looks like:

exports.default = bundle;
// 'module.exports' instead of direct export of the function is used, to
// maintain backward compatibility with Node.js projects, that use CJS module
// system.
module.exports = bundle; 

in my code I can do const bundle = require('@asyncapi/bundler'); and it works
and on local tests with import bundle from '../../src'; also work

@aeworxet aeworxet force-pushed the fix-add-example-with-require-and-alter-exports-in-index.ts-#81 branch from 4ed7eae to 2dd653e Compare October 25, 2022 13:54
@aeworxet
Copy link
Collaborator Author

Then I seem to have overengineered a bit.

@aeworxet aeworxet changed the title fix: change bundle export to CJS syntax and add example with require fix: add module.exports CJS syntax, add example with require Oct 25, 2022
Copy link
Member

@magicmatatjahu magicmatatjahu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also tested on my machine and I see:

exports.default = bundle;
// 'module.exports' is added to maintain backward compatibility with Node.js
// projects, that use CJS module system.
module.exports = bundle;

and it works for ESM and CJS 🚀

tsconfig.json Outdated Show resolved Hide resolved
@aeworxet aeworxet force-pushed the fix-add-example-with-require-and-alter-exports-in-index.ts-#81 branch from 2dd653e to 46e265b Compare October 25, 2022 14:17
Copy link
Member

@magicmatatjahu magicmatatjahu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two changes :)

example/bundle-cjs.js Show resolved Hide resolved
magicmatatjahu
magicmatatjahu previously approved these changes Oct 25, 2022
@derberg
Copy link
Member

derberg commented Oct 25, 2022

@aeworxet please do not do force-push

  • it is hard to check what you changed and reviewer needs to go through entire PR again
  • GitHub Actions do not like it, and as you can see tests were not triggered

…ncapi#81)

Co-authored-by: Maciej Urbańczyk <[email protected]>
Co-authored-by: Lukasz Gornicki <[email protected]>
Copy link
Member

@magicmatatjahu magicmatatjahu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! :)

@magicmatatjahu
Copy link
Member

@derberg ping :)

@aeworxet
Copy link
Collaborator Author

Summoning @Souvikns

Copy link
Member

@Souvikns Souvikns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

@derberg
Copy link
Member

derberg commented Oct 26, 2022

/rtm

@asyncapi-bot asyncapi-bot merged commit 3a16720 into asyncapi:master Oct 26, 2022
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 0.3.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@aeworxet aeworxet deleted the fix-add-example-with-require-and-alter-exports-in-index.ts-#81 branch October 26, 2022 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document how to use package with require
5 participants