Hotfix for missing bigNumberValue on ethValue while creating proposal from template

[mudrila]

No description provided.

[netlify]

✅ Deploy Preview for fractal-prod ready!

Name	Link
🔨 Latest commit	e3ff66b
🔍 Latest deploy log	https://app.netlify.com/sites/fractal-prod/deploys/66151c1b5fc6b7000960ac49
😎 Deploy Preview	https://deploy-preview-1528.app.fractalframework.xyz
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[tomstuart123]

Worked to create proposal templates that can pass successfully at this DAO - https://deploy-preview-1528.app.fractalframework.xyz/proposals/0x77798015f29b314c2afd9b2f307d879dde52957ab1033e8afd5e6ebe14522429?dao=sep:0x74DDE4E0628fad158fC2Bf9772865bd02d9D69FB

[adamgall]

Thanks for this PR Kirill. Does the bigNumberValue property spread amongst these changes still need to have the undefined type? Are there explicit uses for each of the three types (null, undefined, BN) this property takes?

[Da-Colon]

Code changes make sense, looks good! +1 Adams question.

[mudrila]

Thanks for this PR Kirill. Does the bigNumberValue property spread amongst these changes still need to have the undefined type? Are there explicit uses for each of the three types (null, undefined, BN) this property takes?

@adamgall
There's no explicit usage of undefined nor null type. I think we don't need to be marking with undefined explicitly - I adjusted it to mark that it is optional param via ? in applicable places.
Changes in this PR are covering all places where it is required. It could be that there are some other places where it is necessary - I've seen bunch of BigNumber | undefined but not quite sure we need null in those places.

[Da-Colon]

That makes sense. I think We can take a look at the typing in other places in later SOL sessions/PRs

[adamgall]

In retrospect, I don't know if I love this BigNumberInput value type being BigNumber | undefined | null, that seems kind of excessive.

I walk back what I said on Slack... maybe it would be cleaner to just treat a "missing" ethValue property in IPFS as meaning "variable ETH value".

Sorry for going back and forth on this. What's your preference?