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

I believe package @angular/forms should depend on package rxjs #17917

Closed
izaera opened this issue Jul 4, 2017 · 6 comments
Closed

I believe package @angular/forms should depend on package rxjs #17917

izaera opened this issue Jul 4, 2017 · 6 comments

Comments

@izaera
Copy link

izaera commented Jul 4, 2017

I'm submitting a ...


[ ] Regression (behavior that used to work and stopped working in a new release)
[x] Bug report 
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

Currently the module @angular/forms/bundles/forms.umd is depending on 'rxjs/observable/forkJoin', 'rxjs/observable/fromPromise', 'rxjs/operator/map' in its AMD definecall. However, its package.json dependencies section is empty which is incorrect.

Expected behavior

I think that it is not failing now because, in the end, everything is bundled in the correct order and the correct rxjs version is found, but still the dependencies declaration is incomplete.

What is the motivation / use case for changing the behavior?

I'm developing an NPM bundler (a la browserify or webpack) for Liferay Portal and we need to rely on package.json dependencies being declared correctly, otherwise it doesn't work because we resolve dependencies in the very last moment: when the page is rendered.

Would it be possible to add such dependency without breaking anything? I can send a pull request if it is necessary :-).

@trotyl
Copy link
Contributor

trotyl commented Jul 4, 2017

@angular/forms has a peerDependency of @angular/core, the later has a peerDependency of rxjs. Whenever you're using @angular/forms, you should be using @angular/core as well, so rxjs would always be considered a peerDependency.

Meanwhile, bundlers like Webpack have no idea what's listed in the package.json, but just look into the physical directories (by default node_modules), please refer to Webpack documentation for more information.

@izaera
Copy link
Author

izaera commented Jul 5, 2017

Mmmm... that makes sense, because we are not processing peerDependencies yet.

I know about the other bundlers but our case is different because we are not creating a single JS file as I said: we must resolve modules during runtime with the help of an AMD loader and for that we need to scan package.json files.

We don't know in advance how modules are going to be resolved because anyone can deploy/undeploy any package in the server in any moment. Also, we need to support portlets, which are modular and must cooperate between them even without knowing each other. It's a more complex architecture than a single webapp 😅.

@izaera
Copy link
Author

izaera commented Jul 5, 2017

BTW, thanks for the prompt response :-)

@trotyl
Copy link
Contributor

trotyl commented Jul 5, 2017

Hi @izaera , the semantic of peerDependencies is that whoever depends on this package should also depends on its peerDependencies as well. So it's responsibility of the package which has @angular/forms as its dependency to also declare rxjs as its dependency as well.

You could send request to that package owner to add rxjs in dependencies field, since that package is violating the rule.

BTW, it's normally impossible for front-end library to use dependencies, as it doesn't make sense for two different versions of same package being bundled into a single app. Please refer to Peer Dependencies for more information.

@izaera
Copy link
Author

izaera commented Jul 6, 2017

@trotyl I see. In this case the package depending on @angular/forms is my own application/portlet and it has rxjs listed as dependency, but due to the way we load the modules that does not seem enough. I have to think about it more carefully.

The fact that we are not writing a pure bundler (due to the fact that we are not bundling an application but a portlet which is like a chunk of a final application that end up being composed on the fly) makes things a bit more complicated.

For example, as you said, it doesn't make sense to have several versions of a package for a single app, but in our scenario it does, because one portlet may use one version of a package and another one may use a different version and still both can be shown in the same page 😱 and have to coexist peacefully.

Anyway, I think we can close this issue as it is clear that my original request was incorrect and it doesn't make sense to add the dependency to @angular/forms. It is something that has to be "fixed" definitely on my side.

Thanks a lot. This conversation has been very useful to me :-).

@izaera izaera closed this as completed Jul 6, 2017
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019
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

2 participants