Skip to content
This repository was archived by the owner on Jan 20, 2019. It is now read-only.

ember-cli should look up for addons in optionalDependencies as well.#105

Merged
Turbo87 merged 10 commits intoember-cli:masterfrom
sharvansprint:master
Mar 29, 2018
Merged

ember-cli should look up for addons in optionalDependencies as well.#105
Turbo87 merged 10 commits intoember-cli:masterfrom
sharvansprint:master

Conversation

@sharvansprint
Copy link
Contributor

@sharvansprint sharvansprint commented Apr 23, 2017

RFC for the proposal that ember-cli could look up for addons in optionalDependencies as well to include it in consuming application.
rendered

@sharvansprint
Copy link
Contributor Author

cc @kellyselden , kindly guide me through the RFC process.

@sharvansprint sharvansprint changed the title ember-cli could look up for addons in optionalDependencies as well. ember-cli should look up for addons in optionalDependencies as well. Apr 24, 2017
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

In general, I agree that we should handle optionalDependencies better than we do. I think the general proposal (that we discover from optionalDependencies when we can resolve them and do not error if we don't) is perfect.

I'm not really in favor of adding any API to ember-cli-build.js for this though..

@kellyselden
Copy link
Member

We discussed this in our meeting today. We are 100% on board with discovering addons in optionalDependencies. We will try to require.resolve them relative to the package.json they are in, and if they exist, use them, otherwise continue without failing. The new API in ember-cli-build.js seemed a little unnecessary to us, and would like to continue the RFC without it. Does all this sound good to you @SharavanaPrasad? Would you mind updating the RFC with the above info? Also, we don't anticipate the implementation to be that difficult or require that much code, would you be interested in implementing as well?

@sharvansprint
Copy link
Contributor Author

@kellyselden Thank you! That sounds exciting!! I am all ready to open a PR for the same. As per the discussion we can work to make ember-cli scan for Optional Dependencies as well and proceed the build even if any of optional dependencies is missing.

@rwjblue , The mail reason I had asked for an API in ember-cli-build.js was to have a control over the environment specific packages I would want to include in ember-cli App. In my opinion since this can be handled at the level of npm install itself (using appropriate flags npm install --no-optional ), I agree to a drop the idea of wanting an API for ember-cli-build.js.

That's Great 👍

@sharvansprint
Copy link
Contributor Author

I have updated the RFC as per our conclusion. Please check if it's good. Will be pushing a PR for this functionality in a couple of days.
// @kellyselden

Copy link
Contributor

@stefanpenner stefanpenner left a comment

Choose a reason for hiding this comment

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

I'm moving this into FCP. That being said, I don't expect any additional changes/concerns.

Given that, submitting a PR is likely a good idea. The remaining bikeshedding will just be code related.

@Turbo87 Turbo87 merged commit 0ffeeef into ember-cli:master Mar 29, 2018
@Turbo87
Copy link
Member

Turbo87 commented Mar 29, 2018

@SharavanaPrasad sorry that it took us so long, we forgot... 🙈

kategengler pushed a commit to kategengler/rfcs that referenced this pull request Jan 19, 2019
ember-cli should look up for addons in optionalDependencies as well.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants