-
Notifications
You must be signed in to change notification settings - Fork 18
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
Generic CDE #179
Generic CDE #179
Conversation
…ad of parsed JSON
paima-funnel/src/index.ts
Outdated
private extensionsValid: boolean; | ||
|
||
constructor( | ||
web3: Web3, | ||
paimaL2Contract: PaimaL2Contract, | ||
extensions: ChainDataExtension[], | ||
extensionsValid: boolean | ||
) { |
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.
ts has a great shortcut for constructors
constructor(
private readonly web3: Web3,
private readonlypaimaL2Contract: PaimaL2Contract,
private readonly extensions: ChainDataExtension[],
private readonly extensionsValid: boolean,
) {
// No body necessary and you can remove the definition from the class (private extensionsValid: boolean)
}
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.
Did not know this, do you mean like this? https://dev.to/satansdeer/typescript-constructor-shorthand-3ibd
Sounds pretty cool, I will go update that (and we might have superfluous constructors in other places as well in that case)
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.
Exactly that! Parameter Properties https://www.typescriptlang.org/docs/handbook/2/classes.html#parameter-properties. Also readonly
keyword is great for the compiler, so it does not get modified , but also for humans - to understand its just data.
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.
Thanks for the suggestion, I still have a lot to learn about tricks like this in TS (as I've only been using JS/TS since last March), so I appreciate it the tips. Should be updated now :)
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.
🎖️
initializationPrefix
toscheduledPrefix