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

@storybook/angular shouldn't depend on zone.js #4641

Closed
emilio-martinez opened this issue Oct 30, 2018 · 8 comments
Closed

@storybook/angular shouldn't depend on zone.js #4641

emilio-martinez opened this issue Oct 30, 2018 · 8 comments

Comments

@emilio-martinez
Copy link
Contributor

emilio-martinez commented Oct 30, 2018

Describe the bug
Currently, @storybook/angular lists zone.js within its dependencies; however, (as far as I can tell) in practice @storybook/angular doesn't actually depend on it. Of course, Angular does but perhaps it's worth removing it as a @storybook/angular dependency to avoid conflicts.

To Reproduce
See https://github.com/storybooks/storybook/blob/v4.0.0/app/angular/package.json#L44.

Expected behavior
@storybook/angular shouldn't depend on zone.js if it doesn't use it.

Screenshots
N/A

Code snippets
N/A

System:

  • OS: MacOS
  • Device: Macbook Pro 2017
  • Browser: Chrome
  • Framework: Angular
  • Addons: none
  • Version: 4.0.0

Additional context
N/A

@kroeder
Copy link
Member

kroeder commented Oct 30, 2018

@igor-dv @storybook/angular does not depend on it. Wasn't it core or something? We already had a discussion about zone.js

@emilio-martinez
Copy link
Contributor Author

Sorry, re-read my description and realized it was very odd/poorly written 😅updated.

@igor-dv
Copy link
Member

igor-dv commented Oct 30, 2018

It's used in polyfills

https://github.com/storybooks/storybook/blob/master/app/angular/src/client/preview/angular-polyfills.js#L53

Do we really need these polyfills, though ?

@kroeder
Copy link
Member

kroeder commented Oct 30, 2018

Well, this time zone.js isn't a dependency of a dependency (like webpack is just a dep of angular core). zone.js is a direct dep of n angular app - maybe we can put zone.js as peer dep and dev dep for development this time?

@igor-dv
Copy link
Member

igor-dv commented Oct 30, 2018

Yep. It should work

@emilio-martinez
Copy link
Contributor Author

Peer dependency makes sense. The way I see it, Storybook should be pulling the version of zone.js already being used in the host Angular project anyway, unless there's a specific task Storybook is using zone.js for that differs from the rest of the project.

@igor-dv
Copy link
Member

igor-dv commented Oct 31, 2018

Wanna PR this ?

emilio-martinez added a commit to emilio-martinez/storybook that referenced this issue Oct 31, 2018
@emilio-martinez
Copy link
Contributor Author

For sure. Opened!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants