polkadot-js / extension

Simple browser extension for managing Polkadot and Substrate network accounts in a browser. Allows the signing of extrinsics using these accounts. Also provides a simple interface for compliant extensions for dapps.
Apache License 2.0
964 stars 403 forks source link

Transition from manifest v2 to v3 #1367

Open TarikGul opened 1 month ago

TarikGul commented 1 month ago

closes: https://github.com/polkadot-js/extension/issues/1316 closes: https://github.com/polkadot-js/extension/issues/310

Summary (BREAKING CHANGES)

The following PR gives support for manifest V3. Below will help document the exact changes that have taken place in this PR and how it affects the interface given to users.

@polkadot/extension-base

The following covers exported items from @polkadot/extension-base.

class Extension

class State

// Old
const state = new State(providers);

// New
const state = new State(providers);
await state.init();

Checklist

TarikGul commented 1 month ago

So far the extension is working fine in chrome with the exception of the following errors:

Error: Invalid url chrome://extensions/?errors=fnpncejekijbiiemffncneiiilejbmbj, expected to start with http: or https: or ipfs: or ipns:
TarikGul commented 1 month ago

Instead of causing breaking changes to extension-base by enforcing async at the top level of State, I am going to just use the callback method for storage.local

Edit: Ahh but this might not work since there is no more persistent background.

Tbaut commented 1 month ago

edit: There was a stray change in the current state to avoid a TypeError: chrome.extension.getURL is not a function in packages/extension-base/src/background/handlers/State.ts

- const NOTIFICATION_URL = chrome.extension.getURL('notification.html');
+ const NOTIFICATION_URL = chrome.runtime.getURL('notification.html');

I had a quick look on Firefox, changing the manifest to make it happy with what I found here which I'm sure you know

  "background": {
    "scripts": ["background.js"]
  },

edit2: In Firefox the extension is then loaded correctly, I could add an account, but no script is injected on pages, so Dapps don't detect the extensions.

On chrome/brave, the next error I encountered in background.js was

ReferenceError: localStorage is not defined

TarikGul commented 1 month ago

@Tbaut Yea the I was able to fix the localStorage error but then reverted my changes because I would like to change the design of the implementation, but yea the localStorage issue will be fixed soon :)

adamdossa commented 3 days ago

Is it expected that this PR will be merged in the near future? We have a downstream dependency for our Polymesh Wallet which requires this in order to move to manifest V3. @Tbaut

TarikGul commented 3 days ago

Is it expected that this PR will be merged in the near future? We have a downstream dependency for our Polymesh Wallet which requires this in order to move to manifest V3. @Tbaut

Yes this is absolutely a priority! My goal is to get it done this week.

Tbaut commented 14 hours ago

Hello there, I tried this branch and documented my tests:

Note that this includes connecting to some websites, and changing the connected accounts.

Somewhat unexpected for normal users

In summary, it's looking amazing :clap: