Skip to content
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

feat: revamp particleAdapter #854

Merged
merged 10 commits into from
Jan 4, 2024
Merged

feat: revamp particleAdapter #854

merged 10 commits into from
Jan 4, 2024

Conversation

TABASCOatw
Copy link
Contributor

As an alternative to the previous PR, this purely updates the adapter to function with the new version of Particle's solana-wallet SDK, restoring standard functionality to the adapter (which it currently lacks due to the outdated structure and solana-wallet version).

This removes the previous social login display customization changes & abides by the 1 SDK rule.

Copy link

socket-security bot commented Dec 4, 2023

No top level dependency changes detected. Learn more about Socket for GitHub ↗︎

Copy link
Collaborator

@jordaaash jordaaash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, much easier to review. Still have some questions, primary about the configuration interface.

packages/wallets/particle/package.json Outdated Show resolved Hide resolved
packages/wallets/particle/package.json Outdated Show resolved Hide resolved
packages/wallets/particle/src/adapter.ts Outdated Show resolved Hide resolved
packages/wallets/particle/src/adapter.ts Outdated Show resolved Hide resolved
packages/wallets/particle/src/adapter.ts Outdated Show resolved Hide resolved
packages/wallets/particle/src/adapter.ts Outdated Show resolved Hide resolved
@TABASCOatw
Copy link
Contributor Author

GM @jordaaash! Anything else you'd like to be done here? Happy to make any other changes; let me know 👍

@TABASCOatw
Copy link
Contributor Author

Just checking in here again @jordaaash

@TABASCOatw
Copy link
Contributor Author

Hey @jordaaash! Happy New Year! Just wanted to bump this and see if there was anything else that I could do here to push it through?

@jordaaash
Copy link
Collaborator

Catching up on this now. Sorry for the delay!

@TABASCOatw TABASCOatw changed the title Updates ParticleAdapter feat: revamp particleadapter Jan 2, 2024
@TABASCOatw TABASCOatw changed the title feat: revamp particleadapter feat: revamp particleAdapter Jan 2, 2024
@TABASCOatw
Copy link
Contributor Author

Catching up on this now. Sorry for the delay!

No problem at all, thank you!

Copy link
Collaborator

@jordaaash jordaaash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, a few small changes left. I enabled the build to run, but it looks broken. Can you commit the pnpm lockfile changes? I also ran the build locally, and it fails with this error:

@solana/wallet-adapter-particle:build: ../../../node_modules/.pnpm/@[email protected]_@[email protected]_@[email protected][email protected]/node_modules/@particle-network/solana-wallet/lib/types/solana-wallet.d.ts(1,16): error TS2305: Module '"@particle-network/auth"' has no exported member 'LoginOptions'.

packages/wallets/particle/src/adapter.ts Outdated Show resolved Hide resolved
packages/wallets/particle/src/adapter.ts Outdated Show resolved Hide resolved
packages/wallets/particle/src/adapter.ts Outdated Show resolved Hide resolved
@TABASCOatw
Copy link
Contributor Author

Okay, a few small changes left. I enabled the build to run, but it looks broken. Can you commit the pnpm lockfile changes? I also ran the build locally, and it fails with this error:

@solana/wallet-adapter-particle:build: ../../../node_modules/.pnpm/@[email protected]_@[email protected]_@[email protected][email protected]/node_modules/@particle-network/solana-wallet/lib/types/solana-wallet.d.ts(1,16): error TS2305: Module '"@particle-network/auth"' has no exported member 'LoginOptions'.

Just pushed changes addressing those additional comments and fixed the build issue - I went ahead and added my pnpm lockfile to the commit to. The build is working locally for me, could you check again on your side?

Additionally, important to note that this change adds an additional dev dependency to the adapter package, @metamask/eth-sig-util, I tried a few workarounds but it isn't saved as a peer dep and instead needs to be included here to avoid that build issue you ran into, the other option would be to ignore it but could cause some issues.

@jordaaash jordaaash merged commit cf3ccf5 into anza-xyz:master Jan 4, 2024
3 checks passed
@jordaaash
Copy link
Collaborator

Followup PR to fix and release this @ #877

@jordaaash
Copy link
Collaborator

Released via #878

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants