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

bower.json bundles angular and has it as a depedency #4789

Closed
john-banks opened this issue Dec 15, 2015 · 9 comments
Closed

bower.json bundles angular and has it as a depedency #4789

john-banks opened this issue Dec 15, 2015 · 9 comments
Milestone

Comments

@john-banks
Copy link

This commit bfd9f08 altered bower.json so there is only one .js file. This means that angular is now loaded into the project twice making it complain and breaking dependency resolution.

It doesn't alter anything else, just needs reverted.

john-banks referenced this issue Dec 15, 2015
This is proposed for two reasons:
1. pulling the JS portion of Ionic into compliance with the bower.json spec ("The entry-point files necessary to use your package. Only one file per filetype."). I am not sure about how to handle the fonts.
2. making build process simpler for use with bower-main-files or bower-installer, as they can read the main section and grab only what is required.
@mlynch
Copy link
Contributor

mlynch commented Dec 15, 2015

Sounds like we should just remove angular as a dependency for our bower package, since we bundle a specific version in anyways?

@john-banks
Copy link
Author

That doesn't work as other angular modules you want to bring in will have angular as a dependency and force you to bring in another version anyway.

The dependency is already set at "angular": "1.4.3" rather than "angular": "~1.4.3" or "angular": ">=1.4", meaning you are being forceful (perhaps overly so) about which version you want used.

@mlynch
Copy link
Contributor

mlynch commented Dec 15, 2015

I don't see how the first commit would break it, since it would still be bringing in a non-versioned instance of Angular, right?

@neslinesli93
Copy link

+1, had to revert to ionic 1.1 because of this

@perrygovier
Copy link
Contributor

A few points for and against.

For: It's preferred to load all of Ionic as one file. Each Ionic version is tightly coupled with specific versions of Angular, and mixing versions is discouraged and unsupported. This forces that, and makes things more efficient at the same time. It may be necessary in the future to ship Ionic with a patched version of the Angular core (unlikely, but possible).

Against: Other libs and addons might require Angular too. This means Angular gets loaded twice.

I suggest we revert bfd9f08 and maintain strict dependency settings. Any reason this is a bad idea?

A side note: We're moving away from bower in general towards NPM. Bower's still supported in v1, but v2 is all NPM.

@john-banks
Copy link
Author

It's preferred to load all of Ionic as one file. Each Ionic version is tightly coupled with specific versions of Angular, and mixing versions is discouraged and unsupported. This forces that, and makes things more efficient at the same time.

In that case the bundled file acts like something provided by CDN not as a bower component. As it stands currently bfd9f08 will break other implementation (mine included). Because you specify a specific version, if you try and use another bower will warn you that ionic is on another and forces the developer to override. In that case your job is done and any mistakes are their own.

I'm interested in the move towards npm as I really dislike how over simplistic bower is yet npm seems intrinsically slow. npm behaves very differently in that dependencies are installed against themselves rather than with bower where they are installed into one directory and resolved into the agreed version. Not sure how that can be managed with this current situation.

@mlynch
Copy link
Contributor

mlynch commented Dec 16, 2015

Sounds like just reverting the commit is a solution for the short term. In the long term we are moving away from bower so I see no need to innovate on it

@simplesmiler
Copy link

@john-banks regarding npm vs bower deps. npm@^3.0 makes the dep tree as flat as possible, and nests only in case of conflicts.

@mlynch
Copy link
Contributor

mlynch commented Dec 17, 2015

I reverted commit bfd9f08. Seeing as this commit was 25 days ago, I see no reason to innovate on our bower strategy that was working fine (not great, just fine). In the future we are killing off bower so let's keep it like this for now and get it right (no bundling angular) when we move fully to npm.

@mlynch mlynch closed this as completed Dec 17, 2015
@mlynch mlynch added this to the 1.2.1 milestone Dec 17, 2015
@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants