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

[refactor] manifest v2 --> manifest v3 #274

Merged
merged 17 commits into from
Mar 1, 2022

Conversation

tyn1998
Copy link
Member

@tyn1998 tyn1998 commented Feb 19, 2022

Mark this PR draft because I want to test github workflow first. And related docs will be added or updated later.

This PR contais a lot lot lot of changes, but I have trouble in spliting them into minor independent PRs. So I plan to use GitHub review PR features to explain the changes at my best.

Description

Part1:
New manifest v2 extesnsions are no longer accpeted by chrome extension store since January 2022, and exsiting MV2 extensions will not be able to updated when January 2023 comes. So just update to MV3.

Part2:
More comfortable developing experience with more accurate HMR & hot-reload support.

Part3:
webpack4 --> webpack5

Part4:
Update docs.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@tyn1998 tyn1998 force-pushed the manifest-v3-refactoring branch from d2008c5 to 4ca56d7 Compare February 20, 2022 08:14
@tyn1998
Copy link
Member Author

tyn1998 commented Feb 20, 2022

Just updated README and CONTRIBUTING. Not finished yet...

@tyn1998 tyn1998 force-pushed the manifest-v3-refactoring branch from a8c2d1e to a98e793 Compare February 21, 2022 06:48
@tyn1998 tyn1998 changed the title manifest v2 --> manifest v3 [refactor] manifest v2 --> manifest v3 Feb 21, 2022
@menbotics menbotics bot added the kind/enhancement Category issues or prs related to enhancement. label Feb 21, 2022
package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
.eslintrc Show resolved Hide resolved
@tyn1998
Copy link
Member Author

tyn1998 commented Feb 21, 2022

I'm tring to explain what I did in this PR by using GitHub review feature.

This is the first time I used the review feature, good luck to me.

.babelrc Show resolved Hide resolved
.gitignore Show resolved Hide resolved
.gitignore Show resolved Hide resolved
README.md Show resolved Hide resolved
README.zh-CN.md Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
.prettierignore Show resolved Hide resolved
Comment on lines +14 to +16
// d3-dsv csvParse require 'unsafe-eval' CSP, which refused by manifest v3
// so temporily alias this package to the modified pakage in src
'd3-dsv': path.resolve(__dirname, 'src/components/DynamicBar/d3-dsv-2.0.0'),
Copy link
Member Author

@tyn1998 tyn1998 Feb 21, 2022

Choose a reason for hiding this comment

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

DynamicBar uses anichart.js, anichart.js uses d3-dsv, d3-dsv's csvParse require 'unsafe-eval' in Content Security Policy, which refused by manifest v3 extension out of security consideration.

Friends from Open Source world gives help: d3/d3-dsv#80.

I modified d3-dsv source code and place it in our ./src to be part of the project's code, then define an alias in webpack.config.js to tell webpack to search this package in our dir rather node_modules dir.

I know it is not elegant, but it works and when new DynamicBar (absolutely not based on anichart.js) is prepared I will remove these workarounds.

src/services/common.ts Show resolved Hide resolved
src/services/common.ts Show resolved Hide resolved
src/services/common.ts Show resolved Hide resolved
src/services/common.ts Show resolved Hide resolved
Comment on lines +207 to +220
options.optimization = {
minimize: true,
minimizer: [
new TerserPlugin({
extractComments: false,
terserOptions: {
// avoid all constructor.name to be 't' after code compression in production mode,
// because the same constructor.name will lead to only one key in Features(a Map() obj),
// which then faild the inject2Perceptor step.
keep_fnames: true,
},
}),
],
};
Copy link
Member Author

@tyn1998 tyn1998 Feb 21, 2022

Choose a reason for hiding this comment

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

"keep_fnames: ture" option is for a really-hard-to-detect problem:
image

As we know, webpack will try its best to minimize the bundles size in build mode and one of its weapons is to substitute long variable names with short ones. Each module share a common name transformation rule because each module has its own name space when imported properly.

The problem in our code is, we import files directly by import './xxx', this makes 6 transformed t to be exposed in same name space and then they conflict.

So keep founction names when building bundles.

Comment on lines -80 to -94

[web-ext](https://github.com/mozilla/web-ext) 将会自动打开 `Chrome` 浏览器并加载 `Hypertrons-crx`. `web-ext` 的配置请参考 [package.json](https://github.com/hypertrons/hypertrons-crx/blob/master/package.json):

```json
{
"webExt": {
"sourceDir": "distribution",
"run": {
"keepProfileChanges": true,
"chromiumProfile": "./test/web-ext-profile",
"startUrl": ["https://github.com/hypertrons/hypertrons-crx"]
}
}
}
```
Copy link
Member Author

Choose a reason for hiding this comment

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

In our project, web-ext helps to open a new chrome instance and load the unpacked extension into the browser after we running the npm script npm run web-ext.

Now we have a new version of contributing quick start, until web-ext is integrated with webpack devServer and only one npm script is needed to start the whole developing environment, I remove this package temporarily and tell contributors to manually load the unpacked extension.

PS: Only ONE manual load is needed.

"start": "node utils/webserver.js",
"watch": "cross-env NODE_ENV=development node utils/build.js",
"watch:mock": "cross-env NODE_ENV=development MOCK=true node utils/build.js",
"web-ext": "web-ext run --target=chromium",
Copy link
Member Author

Choose a reason for hiding this comment

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

remove web-ext npm script.

Comment on lines -77 to -91

[web-ext](https://github.com/mozilla/web-ext) would open `Chrome` and load `Hypertrons-crx` into the browser automatically. And the default configuration of `web-ext` can be found in [package.json](https://github.com/hypertrons/hypertrons-crx/blob/master/package.json):

```json
{
"webExt": {
"sourceDir": "distribution",
"run": {
"keepProfileChanges": true,
"chromiumProfile": "./test/web-ext-profile",
"startUrl": ["https://github.com/hypertrons/hypertrons-crx"]
}
}
}
```
Copy link
Member Author

Choose a reason for hiding this comment

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

remove introduction to web-ext in README.

"tslint": "^6.1.3",
"tslint-loader": "^3.5.4",
"typescript": "^4.2.2",
"web-ext": "^5.5.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

remove web-ext package.

Comment on lines -160 to -164
new CrxWebpackPlugin({
keyFile: 'build.pem',
contentPath: 'build',
outputPath: 'release',
name: 'hypercrx',
Copy link
Member Author

Choose a reason for hiding this comment

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

crx-webpack-plugin is a webpack plugin to pack an unpacked extension into a packed .crx extension.
When do we need to pack extension into the form of a .crx file? It is when we release it. So this lines of config should not be shown here, but, they are ought to be run in the build script only. In development phase, this packing step costs time but is useless.

So remove from here.

Comment on lines +14 to +22
config.plugins.push(
new CrxWebpackPlugin({
keyFile: path.resolve(__dirname, '../build.pem'),
contentPath: path.resolve(__dirname, '../build'),
outputPath: path.resolve(__dirname, '../release'),
name: 'hypercrx',
})
);

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the right place to setup the plugin.

CrxWebpackPlugin.prototype.apply = function (compiler) {
var self = this;
self.logger = compiler.getInfrastructureLogger('crx-webpack-plugin');
return compiler.hooks.done.tap('crx-webpack-plugin', function () {
Copy link
Member Author

Choose a reason for hiding this comment

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

crx-webpack-plugin does not support webpack5 currently because the author seems not active in this project for a long time. But there is just one API change required to make all things work again, so I modify the source code and put it into our utils dir.

"clean-webpack-plugin": "^3.0.0",
"copy-webpack-plugin": "^5.1.1",
"cross-env": "^7.0.3",
"crx-webpack-plugin": "^0.1.6",
Copy link
Member Author

Choose a reason for hiding this comment

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

Since I have a updated version of crx-webpack-plugin in project's source, here I remove the package.

"babel-loader": "^8.2.3",
"clean-webpack-plugin": "^4.0.0",
"copy-webpack-plugin": "^7.0.0",
"crx": "^5.0.1",
Copy link
Member Author

Choose a reason for hiding this comment

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

crx-webpack-plugin depends on crx, so add this package.

Comment on lines +96 to +100
### HMR & auto-reload

If you are developing Options page or Popup page, each time you save files the pages will hot replace the modules without refreshing, which means you can see the changes right away.

However, if you are developing Background or ContentScripts, each time you save files the service worker will reload the extension automatically. And if you are developing ContentScripts, then pages that injected with ContentScripts will refresh themselves to run the newest scripts.
Copy link
Member Author

Choose a reason for hiding this comment

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

In Chrome extension development, some entries does not support HMR (Hot Module Replacement). Two typical examples are the Content Scripts (which lack the permission to establish a connection with the Dev Server to receive new changes) and the Background script (which is now a service worker under manifest v3 and has no global document node that the HMR requires to work).

One possible solution to achieve a similar effect is to inject clients in the Background script and the Content Scripts and set-up a customized middleware in the Dev Server. The client in the Background script establishes a long connection with the Dev Server. When the Dev Server announces a code change in the Background scripts, the client in the Background scripts reload the entire extension with chrome.runtime.reload(); when the Dev Server announces a code change in the Content Scripts, the client in the Background script communicate with the client in the Content Scripts with chrome.runtime.sendMessage(), and the client in the Content Scripts refresh the page. This Auto-Reload and Auto-Refresh approach makes the new changes in the code to be applied immediately and eliminates some tiny troubles. However, unlike HMR, which can preserve state of the code, this approach will inevitably lose all state of the code, which can be unwanted in some cases.

Notice that the background service worker will be inactive automatically after a period of time, which will cause both the Auto-Reload and Auto-Refresh functionality to be non-functional. One possible solution to fix this issue is to keep the DevTool panel of the Background script open during the development

Comment on lines +1 to +66
const querystring = require('querystring');

const logger = (msg) => {
console.log(`[BGC] ${msg}`);
};

logger('background client up.');

logger('connecting to SSE service...');
const port = querystring.parse(__resourceQuery.slice(1)).port;
const es = new EventSource(`http://localhost:${port}/__server_sent_events__`);

es.addEventListener(
'open',
() => {
logger('SSE service connected!');
},
false
);

es.addEventListener(
'error',
(event) => {
if (event.target.readyState === 0) {
console.error('[BGC] you need to open devServer first!');
} else {
console.error(event);
}
},
false
);

es.addEventListener('background-updated', () => {
logger("received 'background-updated' event from SSE service.");
logger('extension will reload to reload background...');
chrome.runtime.reload(); // reload extension to reload background.
});

es.addEventListener(
'content-scripts-updated',
() => {
logger("received 'content-scripts-updated' event from SSE service.");
chrome.tabs.query({}, (tabs) => {
tabs.forEach((tab) => {
chrome.tabs.sendMessage(
tab.id,
{
from: 'backgroundClient',
action: 'reload-yourself',
},
(res) => {
if (chrome.runtime.lastError && !res) return;

const { from, action } = res;
if (from === 'contentScriptClient' && action === 'yes-sir') {
es.close();
logger('extension will reload to update content scripts...');
chrome.runtime.reload();
}
}
);
});
});
},
false
);
Copy link
Member Author

Choose a reason for hiding this comment

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

backgroundClient.js will be bundled with Background page with webpack. In development mode, you should find this client code is injected into background.bundle.js.

What backgroundClient.js does is trying to establish a long connection to the devServer, so devServer can send messages to the client directly.

The messages include informations like "Background code was updated" or "ContentScripts is updated", according to which the service worker reloads the extension or tells pages that run the ContentScripts to refresh themselves.

This is how auto-reload for Background and ContentScirpts works (client side).

Comment on lines +1 to +16
const logger = (msg) => {
console.log(`[CSC] ${msg}`);
};

logger('content script client up.');

chrome.runtime.onMessage.addListener((request, _sender, sendResp) => {
const shouldReload =
request.from === 'backgroundClient' && request.action === 'reload-yourself';
if (shouldReload) {
sendResp({ from: 'contentScriptClient', action: 'yes-sir' });
// wait 100ms for extension reload.
logger('page will reload to reload content script...');
setTimeout(() => window.location.reload(), 100);
}
});
Copy link
Member Author

Choose a reason for hiding this comment

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

This contentScriptClient.js receives messages from backgroundClient.js to refresh the page when ContentScripts updated.

Comment on lines +70 to +161
setupMiddlewares: (middlewares, devServer) => {
// if auto-reload is not needed, this middleware is not needed.
if (
!customOptions.enableBackgroundAutoReload &&
!customOptions.enableContentScriptsAutoReload
) {
return middlewares;
}

if (!devServer) {
throw new Error('webpack-dev-server is not defined');
}

// imagine you are using app.use(path, middleware) in express.
// in fact, devServer is an express server.
middlewares.push({
path: '/__server_sent_events__', // you can find this path requested by backgroundClient.js.
middleware: (req, res) => {
const sseStream = new SSEStream(req);
sseStream.pipe(res);

sseStream.write('message from webserver.');

let closed = false;

const compileDoneHook = debounce((stats) => {
const { modules } = stats.toJson({ all: false, modules: true });
const updatedJsModules = modules.filter(
(module) =>
module.type === 'module' &&
module.moduleType === 'javascript/auto'
);

const isBackgroundUpdated = updatedJsModules.some((module) =>
module.nameForCondition.startsWith(
path.resolve(__dirname, '../src/pages/Background')
)
);
const isContentScriptsUpdated = updatedJsModules.some((module) =>
module.nameForCondition.startsWith(
path.resolve(__dirname, '../src/pages/ContentScripts')
)
);

const shouldBackgroundReload =
!stats.hasErrors() &&
isBackgroundUpdated &&
customOptions.enableBackgroundAutoReload;
const shouldContentScriptsReload =
!stats.hasErrors() &&
isContentScriptsUpdated &&
customOptions.enableContentScriptsAutoReload;

if (shouldBackgroundReload) {
sseStream.writeMessage(
{
event: 'background-updated',
data: {}, // "data" key should be reserved though it is empty.
},
'utf-8'
);
}
if (shouldContentScriptsReload) {
sseStream.writeMessage(
{
event: 'content-scripts-updated',
data: {},
},
'utf-8'
);
}
}, 1000);

const plugin = (stats) => {
if (!closed) {
compileDoneHook(stats);
}
};

// a mini webpack plugin just born!
// this plugin will be triggered after each compilation done.
compiler.hooks.done.tap('extension-auto-reload-plugin', plugin);

res.on('close', () => {
closed = true;
sseStream.unpipe(res);
});
},
});

return middlewares;
},
Copy link
Member Author

Choose a reason for hiding this comment

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

Add a middleware to webpack devServer (which basicly is a Express server).

This middleware handles the EventSrouce request from backgroundClient, and start a long connection. There is also a mini webpack plugin implemented here to watch the webpack compilation event so each time Background or ContentScripts related code is changed, the devServer can send events to backgroundClient.

Then backgroundClient makes auto-reload happen in browser.

Comment on lines +57 to +58
enableBackgroundAutoReload: true, // always true when "enableContentScriptsAutoReload" is set true
enableContentScriptsAutoReload: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

auto-reload feature also can be disabled

package.json Show resolved Hide resolved
"jsx": "react"
"noEmit": false,
"jsx": "react",
"experimentalDecorators": true,
Copy link
Member Author

Choose a reason for hiding this comment

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

Enable "experimentalDecorators" to use @runwhen decorators.

@tyn1998 tyn1998 marked this pull request as ready for review February 22, 2022 12:42
@tyn1998
Copy link
Member Author

tyn1998 commented Feb 22, 2022

@frank-zsy @LiuChangFreeman @heming6666
Hi guys, I've made some changes in this PR and written comments to explain why I made these proposed changes. Can you spend some time to review them?

@heming6666
Copy link
Contributor

LGTM.

@tyn1998 Good work!

@tyn1998
Copy link
Member Author

tyn1998 commented Feb 26, 2022

@heming6666 Thank you for reviewing!

@heming6666
Copy link
Contributor

Thanks @tyn1998 for your great work! These changes will make this project be better and more efficient.

We can wait further reviews from @frank-zsy @LiuChangFreeman until 2022/3/1 (next MONDAY).

@zhuxiangning
Copy link
Contributor

/approve

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Category issues or prs related to enhancement. pull/approved If a pull is approved, it will be automatically merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants