Issues with account portfolio code

[bayareacoder]

It seems to me there are a few issues inPortfolio.ts to retrieve an account's portfolio:

• init issues reqAccountUpdates and listens for updatePortfolio.
• on each updatePortfolio (which is a callback for each position in the portfolio), there is a nested call to self.getPortfolios which by itself then issues reqPositions with separate listeners.

Per: http://interactivebrokers.github.io/tws-api/account_updates.html, reqAccountUpdates will already receive updatePortfolio for each position in the account, and then a accountDownloadEnd when all data is received (for which there is no listener in the current code). So it seems to me there is no need to also use reqPositions.

It's not clear to me how to make a distinction between an initial snapshot and later updates to the portfolio as there is only 1 event topic IBKREVENTS.PORTFOLIO. So how to know if it's a full portfolio download or an update with only a change?

I also wouldn't think you need separate listeners for filled orders, since I'd expect that would trigger updatePortfolio callback automatically if a subscription is setup, see below.

Per #66 accountId should also be an input to Portfolio class to support clients with multiple accounts so you can retrieve the portfolio per account (probably passing accountId in the Portfolio constructor so you get a portfolio instance per accountId will be easiest, and then can do a subscription per accountId), see next point.

You should be able to either only get a full snapshot of the portfolio, or do a subscription so that after an initial snapshot you continue to get portfolio updates, and then be able to stop that subscription at a later time.

PS I would call the class Portfolio instead of Portfolios if implemented per above (a portfolio assumes there are multiple positions).

I'm working on a re-write.

[ceddybi]

@bayareacoder thank you very much for your findings,

I did a rough PR for this fix, https://github.com/stoqey/ibkr/pull/67

• To my understanding for the reqPositions method that we call can be replaced with reqPositionsMulti with takes the accountId as an argument,

Please have a look and let me know what you think about this fix,

PS: and I def agree with you on the naming, we have to change Portfolio to Portfolios, and there are more improvements that we should add too,

Please kindly create a PR, and use my demo PR for reference and I just set up some discussion on here https://github.com/stoqey/ibkr/discussions/69 for future discussions if required

Thank. you

[bayareacoder]

Here is my re-write of the Portfolio code. This resolves:

• ability to now download portfolio per accountId
• captures both accountValue and portfolio that is returned by reqAccountUpdates
• allows to keep receiving updates after initial snapshot
• communicates everything via events
• no extensive TS types for the returned data; just use a Record<> type
• groups account data by currency (could have different currencies returned for a single account in principle - the IB API allows for this so they could do eg margin calculations by currency) and portfolio by contract ID (dangerous to group by symbol I think since you could have >1 contract for the same symbol)
• all typescript types in a separate types.ts file, so they can be imported separately only by users who use TS
• added a file that can be used for help text in a UI for all fields, indexed by object key (more efficient than array search)

Re-writing more of this library based on the original @pilwon/node-ib library. Not sure I'll have time to do a PR and may not make sense since we're redoing the complete interface to better fit our needs. But hopefully these ideas can help you for the maintenance of your library.

Portfolio.ts: https://gist.github.com/bayareacoder/d033493c6920256c037a63102f5cf66f

types.ts: https://gist.github.com/bayareacoder/cc5688e67b7307a279b9b7d296b167c0

accountValue.const.ts: https://gist.github.com/bayareacoder/9cbd59a628ccbe01d187bd2ead440230

[ceddybi]

@bayareacoder Thank you very much for your code, please do note that IBKR is heavily dependent on https://github.com/stoqey/ib which is a total new re-write of the @pilwon/node-ib library,

Please create a PR with your new code and I'll be more than happy to push it forward

Thank you

[bayareacoder]

@ceddybi Actually yes I meant @stoqey/ib, which is what we're using. I noticed it has support for conditional orders, news etc. and looks quite solid. The TS typings in it are very useful.

[ceddybi]

@bayareacoder Super awesome, am very happy you're satisfied with @stoqey/ib, as this library IBKR is far behind from IB,

Do you mind closing all other issues and using IB instead?

[bayareacoder]

The 2 issues I reported with this library should remain open I believe as long as there is no new release that fixes them, as a warning to others who stumble on this. While this is a good start as a higher-level lib, and so appealing to use, it seems also still unfinished and work in progress. If someone wants to start from this code, instead of the lower-level @stoqey/ib, they should be aware.

[ceddybi]

fixed from main