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

Added ember-addon support via npm #2238

Merged
merged 1 commit into from
Sep 5, 2014
Merged

Conversation

jayphelps
Copy link
Contributor

Worked with @rwjblue on this. To publish to npm, you'll need ember-cli v0.0.43 that has this PR which as of this writing means canary.

@rwjblue
Copy link
Member

rwjblue commented Sep 5, 2014

👍

"scripts": {
"postinstall": "bower install",
"prepublish": "rm -rf dist && ember build --output-path=dist/ember-data/ --environment production",
Copy link
Member

Choose a reason for hiding this comment

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

rm -rf dist is not needed (ember-cli clears the output dir before copying the new build in).

@fivetanley
Copy link
Member

I feel hesitant about this. Does this mean we'll publish this package to npm without it working in browserify? That seems misleading to browserify users.

@fivetanley
Copy link
Member

I worry about having to bump minor versions and they won't line up between bower and npm and rubygems for fixes for the ember-cli loader. My personal preference is that ember-cli stuff lives in its own place like it currently does, due to its changing nature. I would prefer the version numbers to reflect the packaging as accurately as possible.

I'm working on npm support this weekend.

Edit: I don't mean to be a downer on the effort though. Ember cli addons are great I'm just not sure this will be the right place for this from a library authoring perspective right now.

@rwjblue
Copy link
Member

rwjblue commented Sep 5, 2014

How much do you enjoy wrapper packages :trollface:?

I can't see how publishing something (where nothing exists today) can harm anyone. We can always add a separate CJS entry point in the future without breaking any compatibility.

@fivetanley - Am I understanding your argument correctly as: "we can't support everyone so we shouldn't support anyone"?

@jayphelps
Copy link
Contributor Author

@rwjblue basically said what I was thinking as well, but I do see that there has to be a line somewhere. We can't support everything. That said, last I heard ember-cli was the de facto official so I think supporting it natively is crucial. Regarding publishing w/o CJS support, it is indeed misleading. But compare the number of ember-cli peeps vs. the number of people who would run into that.

There's also this...https://www.npmjs.org/package/ember

@rwjblue
Copy link
Member

rwjblue commented Sep 5, 2014

@jayphelps - To get ember-cli to build correctly into a nested directory you will need to update the version to 0.0.43 (which should work without other major changes).

@fivetanley
Copy link
Member

EDIT: NVMIND LOL

@stefanpenner
Copy link
Member

@fivetanley countless wrapper packages is the ghetto. We can role out NPM support incrementally.

@fivetanley
Copy link
Member

So on the drive home I realized how right you all are because if worse comes to worse we could move the new lib/ember-addon-main.js file to a separate package.

Forgive my slowness. :P

@fivetanley
Copy link
Member

👍 from me

@jayphelps
Copy link
Contributor Author

@fivetanley
I FRIGGIN LOVE YOU

fivetanley added a commit that referenced this pull request Sep 5, 2014
Added ember-addon support via npm
@fivetanley fivetanley merged commit 4b0379c into emberjs:master Sep 5, 2014
@jayphelps
Copy link
Contributor Author

One of you maintainers should prolly be the one to npm publish. Do you guys need a ticket for a reminder or nope?

@stefanpenner
Copy link
Member

Ticket please

@jayphelps
Copy link
Contributor Author

NO TICKET

@jayphelps jayphelps mentioned this pull request Sep 6, 2014
2 tasks
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.

4 participants