-
Notifications
You must be signed in to change notification settings - Fork 3k
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
docs: introduce importing document #6548
docs: introduce importing document #6548
Conversation
@benlesh any comments on this one? |
@jakovljevic-mladen this is a very detailed document. I think one thing I would like to see if that directly at the top as bold as possible, we state very simply and plainly as possible that as of 7.2, no one should be importing things from Like, I wouldn't make people scroll or read too far at all before they saw that. It should be bold and obvious. |
Got it, will add. How about code examples in the docs? This isn't updated anywhere. I was thinking about adding some PRs to update this, but import from import { fromEvent } from 'rxjs';
import { filter } from 'rxjs/operators'; to this: import { fromEvent, filter } from 'rxjs';
// If you're using RxJS v7.1 or lower, use this import
// import { fromEvent } from 'rxjs';
// import { filter } from 'rxjs/operators'; What do you think about doing it this way? Or I should just do this: import { fromEvent, filter } from 'rxjs'; |
Yeah. This needs updated everywhere. |
We do have the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for writing a document about this!
I created a seperated ticket for updating the imports in the code example. I guess it would be the best to do this in a separate PR as it will be quite bloated probably. |
Updated, it looks like this now: It's not the first section in the document, but it's still visible without scrolling. Is this OK? And, yes, I used
I was thinking about doing it in separate PRs, not this one. Thanks for creating an issue for this. |
9bf7732
to
4691714
Compare
Description:
As of RxJS version 7.2.0, a new way of importing operators is introduced. This PR adds a document that covers this addition.
Related issue (if exists):
None