Skip to content

chore: next-gen app #259

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

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

chore: next-gen app #259

wants to merge 15 commits into from

Conversation

meeh0w
Copy link
Member

@meeh0w meeh0w commented May 12, 2025

Changes

  1. Splits the codebase into Yarn workspaces to make it easier to develop, build and test two versions of the extension.
  2. No real logic changes were introduced in the files (some imports may have been renamed, but the logic should be the same).
  3. Makes sure any references to @avalabs/core-k2-components are not leaked to the new app via some shared hooks or react contexts.
  4. Sets up apps/next directory with basic structure, a welcoming screen and a simple unit test

Directories breakdown

  • background script was moved to packages/service-worker (@core/service-worker)
  • inpage script was moved to packages/inpage (@core/inpage)
  • content script was moved to packages/content-script (@core/content-script)
  • offscreen document was moved to packages/offscreen (@core/offscreen)
  • shared types landed in packges/types (@core/types)
    • this part I don't really like anymore. I underestimated how many there are and will move them back next to their respective services/handlers, but this will take a while, so I'm not doing this in this PR (I was too far down already)
  • things shared by multiple packages above landed in packages/common (@core/common)
  • things that are used by the legacy app and will likely be used by the new app (react contexts, hooks) landed in packages/ui (@core/ui)
  • the legacy app itself (UI, assets, manifest files) landed in apps/legacy (@core-ext/legacy)
  • packages/tsconfig holds the base tsconfig file and some widely used .d.ts files
├── apps/
  ├── legacy
  ├── next
├── packages/
  ├── common
  ├── content-script
  ├── inpage
  ├── messaging
  ├── offscreen
  ├── service-worker
  ├── tsconfig
  ├── types
  ├── ui

Scripting

  • yarn workspaces foreach command is widely used.
  • Not every package is buildable (i.e. @core/ui, @core/common are included as-they-are by the buildable packages that use them, e.g. the service worker)
  • All existing scripts (yarn dev, yarn build, etc.) should still work and build the entire legacy app.
  • All buildable packages (offscreen, inpage, service-worker, content-script) have their output set to the dist/<namespace> folder (so they can clear it when rebuilding without worrying about each others' files).
    • The legacy app builds directly into the dist folder, does not touch the <namespace> sub-directories.

Testing

  • Check out the branch
  • Rename your .env file to .env.dev (there is a new env variable in 1pass - NEXT_GEN_EXTENSION_PUBLIC_KEY)
  • All scripts should still work (yarn install, yarn dev, yarn build, yarn typecheck, yarn scanner, etc.)
  • Extension ID should not change
  • The app should work as before.
  1. yarn dev:next
  2. Go to chrome://extensions, click Load unpacked and point to dist-next directory
  3. You should see an empty welcome screen

Screenshots:

Screen.Recording.2025-05-12.at.13.27.27.mov

Checklist for the author

  • I've covered new/modified business logic with Jest test cases.
  • I've tested the changes myself before sending it to code review and QA.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates configuration files for the Next.js app, refactors toast integrations in various authentication components, and standardizes the imports for the scoped toast utility across legacy pages and hooks.

  • Added jest and i18next scanner configuration
  • Updated components and hooks to pass and import toast from new locations
  • Minor refactoring in authentication flows and renaming utilities

Reviewed Changes

Copilot reviewed 102 out of 104 changed files in this pull request and generated no comments.

Show a summary per file
File Description
apps/next/jest.config.js New Jest configuration for testing with ts-jest and custom resolver setup.
apps/next/i18next-scanner.config.js New i18next scanner config with TypeScript support and custom resource options.
apps/next/README.md Minimal README update for the Next.js app.
apps/legacy/src/popup/popup.tsx Added toast prop to Onboarding and Swap context providers.
apps/legacy/src/pages/Onboarding/pages/Seedless/modals/FIDOModal.tsx Integrated toast into useSeedlessActions and maintained device naming state.
apps/legacy/src/pages/Onboarding/pages/Seedless/modals/AuthenticatorModal.tsx Passed toast to useSeedlessActions for improved error handling.
apps/legacy/src/pages/Onboarding/pages/Seedless/components/GoogleButton.tsx Updated useSeedlessActions call to include toast for signIn.
apps/legacy/src/pages/Onboarding/pages/Seedless/components/AppleButton.tsx Updated useSeedlessActions call to include toast for signIn.
apps/legacy/src/pages/Onboarding/pages/Seedless/RecoveryMethods.tsx Updated useSeedlessActions call to include toast for loginWithoutMFA.
apps/legacy/src/pages/ImportPrivateKey/ImportPrivateKey.tsx Changed the import path for useScopedToast.
apps/legacy/src/pages/Accounts/hooks/useWalletRename.tsx Updated toast import to new scoped path.
apps/legacy/src/pages/Accounts/hooks/useEntityRename.tsx Updated toast import to new scoped path.
apps/legacy/src/pages/Accounts/hooks/useAccountRename.tsx Updated toast import to new scoped path and integrated rename handling.
apps/legacy/src/pages/Accounts/hooks/useAccountRemoval.tsx Updated toast import to new scoped path.
apps/legacy/src/pages/Accounts/components/AccountItem.tsx Updated toast import to new scoped path.
apps/legacy/src/pages/Accounts/Accounts.tsx Updated toast import to new scoped path.
apps/legacy/src/pages/Accounts/AccountDetailsView.tsx Updated toast import to new scoped path.
Files not reviewed (2)
  • .yarn/patches/@mui-material-npm-6.4.11-84c49ed24a.patch: Language not supported
  • apps/legacy/package.json: Language not supported
Comments suppressed due to low confidence (2)

apps/legacy/src/pages/Onboarding/pages/Seedless/modals/FIDOModal.tsx:30

  • Consider renaming 'FIDODeviceName' to 'fidoDeviceName' to adhere to standard lowerCamelCase naming conventions for variables.
const [FIDODeviceName, setFIDODeviceName] = useState('');

apps/legacy/src/pages/Accounts/hooks/useAccountRename.tsx:4

  • The onFailure callback currently uses toast.success for a failure scenario. It should use toast.error to accurately reflect the error state.
const onFailure = useCallback(() => toast.success(t('Renaming failed'), { duration: 1000 }), [toast, t]);

bferenc
bferenc previously approved these changes May 13, 2025
Copy link
Contributor

@bferenc bferenc left a comment

Choose a reason for hiding this comment

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

tested both legacy and next versions 👍

@meeh0w meeh0w changed the base branch from chore/restructure to main May 22, 2025 12:36
@meeh0w meeh0w dismissed bferenc’s stale review May 22, 2025 12:36

The base branch was changed.

@meeh0w meeh0w mentioned this pull request May 22, 2025
2 tasks
@semgrep-code-ava-labs
Copy link

Semgrep found 2 ssc-b5cd4801-a3c5-43d5-bca5-c7fdef10f97b findings:

  • packages/service-worker/src/services/featureFlags/FeatureFlagService.ts

Risk: Affected versions of semver are vulnerable to Inefficient Regular Expression Complexity. The vulnerability exists in the package semver allowing Regular Expression Denial of Service (ReDoS) through the new Range when untrusted user data is input as a range.

Fix: Upgrade this library to at least version 7.5.2 at core-extension/yarn.lock:31424.

Reference(s): GHSA-c2qf-rxjj-qqgw, CVE-2022-25883

Copy link
Contributor

@vvava vvava left a comment

Choose a reason for hiding this comment

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

works fine 👍

Copy link
Contributor

@csabbee csabbee left a comment

Choose a reason for hiding this comment

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

awesome work, thanks!
-

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.

5 participants