-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Fix filter polling leak #1474
Fix filter polling leak #1474
Conversation
Regarding #1458 Uses a new streaming subprovider architecture on an experimental branch of StreamProvider: https://github.com/flyswatter/web3-stream-provider/tree/StreamSubprovider
app/scripts/lib/inpage-provider.js
Outdated
const filterSubprovider = new FilterSubprovider() | ||
engine.addProvider(filterSubprovider) | ||
|
||
const stream = self.stream = new StreamSubprovider() |
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.
can you rename to streamProvider
or something less generic
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.
Like RemoteStreamSubprovider
? I'm not sure what you mean less generic. Isn't descriptive good here?
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.
i mean instead of stream
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.
Oh ok, done.
Good changes! 👍 |
By injecting a
FilterSubprovider
into theinpage-provider
, I'm able to keep filter polling logic in-page, while keeping caching logic in our background process, so that subscriptions are deallocated when pages are closed.Fixes #1458
Required refactoring the
StreamingSubprovider
to be a composableProviderEngine
instance, so that theFilterSubprovider
could be added to it.For this reason, there was a small API break in
web3-stream-provider
that needs to be merged & deployed before these tests can be rebuilt & will pass:MetaMask/web3-stream-provider#1