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

Adds a CommonJS build script #23

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

Conversation

jeffkole
Copy link

This is a start at an attempt to produce a CommonJS build in response to my question yesterday: #22. Let me know what you think.

This works for me, because I am pushing these CommonJS builds to my own Github
repo. If we wanted a proper CommonJS distribution, then it would need to be
published to npm.
@knomedia
Copy link
Contributor

@jeffkole thanks for this, I like it. I wonder if it wouldn't be better to combine this into a single build and a single release (or at least have a single command for each). i can see a future issue where only globals get built and released (or only the commonjs version).

thoughts?

@jeffkole
Copy link
Author

One build and release script is definitely better. The current one is fairly specialized (ie, works great with Browserify and if you are including React globally), so I didn't want mess with it, because I do not know the project history. I mostly stole my solution from what I saw from @ryanflorence in rackt/react-router. It is possible that the CommonJS solution can also work just fine for the Browserify case you had targeted originally.

@knomedia
Copy link
Contributor

yeah, i think it can. we can ditch the browserify solution if we can make something that will work for:

  • users who are not using a module system
  • users who are using requirejs
  • the new CommonJS build you are proposing

@jeffkole
Copy link
Author

#26 may fit the bill better.

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

2 participants