-
Notifications
You must be signed in to change notification settings - Fork 19
Package/avail wallet #106
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
base: feat/wallet-sdk
Are you sure you want to change the base?
Package/avail wallet #106
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
PR Summary
Major restructuring of the avail-wallet package, transitioning from Rollup to Vite build system with updated TypeScript configurations and component architecture.
- Removed critical type declarations in
src/declarations.d.ts
andsrc/css.d.ts
which could break image imports and CSS module typing - Switched from
src/index.ts
tosrc/lib/index.ts
as main export, potentially affecting package's public API - Removed
utils.ts
containing essential metadata and API initialization functions without clear replacement - Incomplete background class in
DialogContent
component (bg-
with no color value) - Hardcoded
test
flag inuseInitialise.ts
makes API initialization non-functional
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
40 file(s) reviewed, 19 comment(s)
Edit PR Review Bot Settings | Greptile
"rsc": false, | ||
"tsx": true, | ||
"tailwind": { | ||
"config": "", |
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.
logic: Empty Tailwind config path could cause build issues. Should point to tailwind.config.js or similar
"prebuild": "rm -rf dist", | ||
"lint": "eslint . --ext ts,tsx --report-unused-disable-directives --max-warnings 0", | ||
"preview": "vite preview", | ||
"prepublish": "npm run build" |
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.
style: Use 'prepare' instead of 'prepublish' as it's deprecated in npm
"prepublish": "npm run build" | |
"prepare": "npm run build" |
// import { AvailWalletConnect } from "avail-wallet"; | ||
|
||
function App() { | ||
return <div className="h-screen">{/* <AvailWalletConnect /> */}</div>; |
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.
style: AvailWalletConnect import and usage are commented out. Either implement the wallet connection or remove the commented code to avoid confusion.
<DialogPrimitive.Content | ||
ref={ref} | ||
className={cn( | ||
"fixed outline-none focus:outline-none focus:ring-0 ring-0 left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%] gap-4 border border-gray-200 bg- p-6 shadow-lg duration-200 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%] sm:rounded-2xl dark:border-gray-800 dark:bg-gray-950", |
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.
syntax: Incomplete background class 'bg-' found in className string - needs a color value
"fixed outline-none focus:outline-none focus:ring-0 ring-0 left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%] gap-4 border border-gray-200 bg- p-6 shadow-lg duration-200 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%] sm:rounded-2xl dark:border-gray-800 dark:bg-gray-950", | |
"fixed outline-none focus:outline-none focus:ring-0 ring-0 left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%] gap-4 border border-gray-200 bg-white p-6 shadow-lg duration-200 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%] sm:rounded-2xl dark:border-gray-800 dark:bg-gray-950", |
|
||
interface AvailWalletContextType { | ||
api?: ApiPromise; | ||
api?: any; |
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.
style: Using 'any' type for api reduces type safety. Consider using ApiPromise type or creating a specific interface.
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.
logic: Removing these font declarations and utility classes may break component styling. Verify that these fonts and classes are either moved elsewhere or no longer needed.
export { | ||
AccountSelector, | ||
AvailWalletConnect, | ||
AvailWalletProvider, | ||
DisconnectWallet, | ||
MetaMaskContext, | ||
MetaMaskProvider, | ||
useApi, | ||
useAvailAccount, | ||
useAvailWallet, | ||
useInvokeSnap, | ||
useMetaMask, | ||
useMetaMaskContext, | ||
useRequestSnap, | ||
WalletSelector, | ||
}; |
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.
style: exports are not alphabetically sorted which is typically a best practice for maintainability
export interface AvailWalletConnectProps { | ||
api?: ApiPromise; | ||
} | ||
children?:ReactNode |
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.
syntax: Missing space after colon in children prop type declaration
children?:ReactNode | |
children?: ReactNode |
@@ -0,0 +1 @@ | |||
{"root":["./src/app.tsx","./src/index.tsx","./src/vite-env.d.ts","./src/components/ui/badge.tsx","./src/components/ui/button.tsx","./src/components/ui/dialog.tsx","./src/components/wallets/accountselector.tsx","./src/components/wallets/availwalletconnect.tsx","./src/components/wallets/availwalletprovider.tsx","./src/components/wallets/disconnectwallet.tsx","./src/components/wallets/walletselector.tsx","./src/hooks/useinitialise.ts","./src/hooks/metamask/metamaskcontext.tsx","./src/hooks/metamask/index.ts","./src/hooks/metamask/types.ts","./src/hooks/metamask/useinvokesnap.ts","./src/hooks/metamask/usemetamask.ts","./src/hooks/metamask/userequest.ts","./src/hooks/metamask/userequestsnap.ts","./src/hooks/metamask/utils/index.ts","./src/hooks/metamask/utils/metamask.ts","./src/hooks/metamask/utils/snap.ts","./src/lib/index.ts","./src/lib/utils.ts","./src/stores/api.ts","./src/stores/availwallet.ts","./src/types/index.ts"],"version":"5.8.3"} No newline at end of file |
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.
logic: File paths use incorrect casing - 'badge.tsx' should be 'Badge.tsx', 'button.tsx' should be 'Button.tsx', etc. This will cause build failures on case-sensitive systems.
name: "avail-wallet", | ||
}, | ||
commonjsOptions: { | ||
include: [/avail-js-sdk/, /node_modules/], |
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.
style: Including all of node_modules in commonjsOptions could impact build performance. Consider limiting to specific packages that need CommonJS transformation.
No description provided.