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

Build CommonJS version of gavel #536

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

realityking
Copy link

This PR adds a CommonJS version of this module for the benefit of Node use-cases including dredd.

TBH I'm not sure why this package is build as an UMD module. I suspect this might be how it's deployed for Apiary UI? In that case this might require some change as the filename of the UMD build changes with this PR.

Not only is CommonJS code generating less closures than UMD, it also doesn't bundle all dependencies. This allows dredd to load, for example, media-typer only once.

I also moved url to be a dev dependency as it's only used to build the UMD version. For Node.js the built-in version of url is preferred.

@realityking
Copy link
Author

Revisiting this PR, it doesn't seem like gavel is build with browsers in mind at all. Is there a reason to ship the UMD version at all? It might be good to only ship the CommonJS build.

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.

None yet

1 participant