-
Notifications
You must be signed in to change notification settings - Fork 9
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
Use only web3 as dependency #1
Comments
So there's actually a misconception regarding the plugin's use of declare module 'web3' {
interface Web3 {
chainlink: ChainlinkPlugin;
}
} I don't think the above code is the responsibility of the plugin, it actually belongs to the user using the To explain this better, consider the following: import { Web3Context } from 'web3-core/dist/web3_context';
import { ChainlinkPlugin } from '../../src/index';
declare module 'web3-core' {
interface Web3Context {
chainlink: ChainlinkPlugin;
}
}
describe('Chainlink Plugin Tests', () => {
it('should register ChainlinkPlugin and make the getPrice call', async () => {
const web3Context = new Web3Context('https://rpc.ankr.com/eth_rinkeby');
web3Context.registerPlugin(new ChainlinkPlugin(AggregatorV3InterfaceABI, aggregatorAddress));
const price = await web3Context.chainlink.getPrice();
expect(Object.keys(price)).toEqual(
expect.arrayContaining([
'roundId',
'answer',
'startedAt',
'updatedAt',
'answeredInRound',
]),
);
});
}); In this example there is no dependency on Aside from the above, I don't think we should rely on the |
Module augmentationWithout plugin author augmenting types, you have two options:
I would suggest that we continue this discussion on some issue or discussion board on web3.js Original issueThis issue is blocked by web3/web3.js#5508 where I moved discussion, |
Importing types and functionality from a bunch of different web3 packages are making it hard to link web3 and test changes against the dev version of web3.
so instead of:
we should only have:
if we can't do that, we should raise an issue on the web3 repo to export missing functionality.
The text was updated successfully, but these errors were encountered: