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

Removed dependencies #9

Merged
merged 1 commit into from
Nov 1, 2015
Merged

Removed dependencies #9

merged 1 commit into from
Nov 1, 2015

Conversation

piotrd
Copy link
Contributor

@piotrd piotrd commented Oct 22, 2015

Removed angular and angular-sanitize dependencies, as they break building project with Browserify if angular is bundled as a non-npm module (Error: Cannot find module 'angular'). Requiring these doesn't have to occur in a plugin, they are usually required somewhere in the app.

Removed `angular` and `angular-sanitize` dependencies, as they break building project with Browserify if `angular` is bundled as a non-npm module (`Error: Cannot find module 'angular'`).
@2fdevs
Copy link
Member

2fdevs commented Nov 1, 2015

Accepted.

Thanks for the suggestion.

2fdevs added a commit that referenced this pull request Nov 1, 2015
@2fdevs 2fdevs merged commit 5533e5f into videogular:master Nov 1, 2015
piotrd added a commit to piotrd/angular-scroll that referenced this pull request Dec 8, 2015
Similar to the following PR: videogular/bower-videogular#9. Placing angular as a dependency in `index.js` causes `WARNING: Tried to load angular more than once. Angular JS`.
@whitneyit
Copy link

@2fdevs, Is it possible to look into this again? I think the issue here was that the dependencies were not listed in package.json file.

The above link points to the package.json at the time that this PR was fixing the broken build, and it appears that the breakage may be simply due to browserify not being able to find the modules because they were not installed into the package.json file.

I want to add videogular to the jspm registry via #726 and this is blocking videogular from being added to the registry.

Re: the relevant comment.

@piotrd
Copy link
Contributor Author

piotrd commented Feb 1, 2016

@whitneyit may be right. I took time to test if the removed lines from this PR break the build if I add them back and it looks like all works fine (tested with the latest Browserify).

@Elecash
Copy link
Member

Elecash commented Feb 1, 2016

Ok, I'm going to add those lines again.

Thanks @whitneyit for the warning and @piotrd for following the conversation :)

2fdevs pushed a commit that referenced this pull request Feb 1, 2016
@Elecash
Copy link
Member

Elecash commented Feb 1, 2016

@whitneyit I've updated the latest tag and the master branch.

@whitneyit
Copy link

Can these changes also be applied to the other bower repos?

Also, it appears that package.json does not have the dependencies added.

$ npm install --save angular angular-sanitize

@Elecash
Copy link
Member

Elecash commented Feb 3, 2016

Yeah, sure!

Should we add to other bower repos videogular as a dependency too?

@whitneyit
Copy link

Yea that sounds like a good idea!

@Elecash
Copy link
Member

Elecash commented Mar 4, 2016

Done!

Sorry for taking so long in doing this!

@whitneyit
Copy link

All good man thanks!

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