Skip to content

Commit

Permalink
extension: switch to rollup for building instead of webpack
Browse files Browse the repository at this point in the history
mainly because

- rollup has a much better builtin ES6 module support (in webpack it's STILL experimental!)
- minimal transfromation to the source code, even possible to read without source maps
  • Loading branch information
karlicoss committed May 24, 2024
1 parent 58f2865 commit 2f78cae
Show file tree
Hide file tree
Showing 18 changed files with 342 additions and 358 deletions.
11 changes: 3 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,6 @@ In addition:
- \+ using a bookmarklet, hence browser-agnostic
- \+ capable of on the fly HTML to org-mode markup conversion

# Requirements
No third party dependencies! Just `python3`.

# Potential improvements
* see [todos](./TODO.org)

Expand All @@ -96,13 +93,11 @@ You need `npm` for building the extension.
After that you can find the extension in `dist` directory and 'Load unpacked** if necessary. There is also Flow and Eslint set up.

## testing and linting
Check [CI config](./.circleci/config.yml) to figure out all the checks I'm doing.

The only test(s) that don't run on CI at the moment (e.g. due to lack of X server) are marked with `@skip_if_ci`. You can run them manually though.
Check [CI config](./.github/workflows/main.yml) to figure out all the checks I'm doing.

Extra tests (not integrated in CI yet):
There are some end2end tests which check both web extension and the browser, but require GUI, so they can't run on github actions. You can run them manually though.

- `scripts/test_with_browser.py`
- `pytest -s --pyargs tests.test_end2end`

## publishing

Expand Down
File renamed without changes.
9 changes: 4 additions & 5 deletions extension/eslint.config.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
// @ts-check
import globals from 'globals'
import eslint from '@eslint/js'
import tseslint from 'typescript-eslint'

const globals = require('globals')
const eslint = require('@eslint/js')
const tseslint = require('typescript-eslint')


module.exports = tseslint.config(
export default tseslint.config(
eslint.configs.recommended,
...tseslint.configs.recommended, // TODO recommendedTypeChecked??
{
Expand Down
180 changes: 180 additions & 0 deletions extension/generate_manifest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
import assert from 'assert'

import pkg from './package.json' with { type: "json" }

const T = {
CHROME : 'chrome',
FIREFOX: 'firefox',
}


// ugh. declarative formats are shit.
export function generateManifest({
target, // str
version, // str
release, // bool
ext_id // str
} = {}) {
assert(target)
assert(version)
assert(release !== null)
assert(ext_id)

const v3 = version == '3'

// Firefox wouldn't let you rebind its default shortcuts most of which use Shift
// On the other hand, Chrome wouldn't let you use Alt
const modifier = target === T.CHROME ? 'Shift' : 'Alt'

const action_name = v3 ? 'action' : 'browser_action'

const commands = {
"capture-simple": {
"description": "Quick capture: url, title and selection",
"suggested_key": {
"default": `Ctrl+${modifier}+H`,
"mac": `Command+${modifier}+H`,
},
},
}

commands[`_execute_${action_name}`] = {
"description": "Capture page, with extra information",
"suggested_key": {
"default": `Ctrl+${modifier}+Y`,
"mac": `Command+${modifier}+Y`,
},
}


const action = {
"default_icon": "img/unicorn.png",
"default_popup": "popup.html",
"default_title": "Capture page, with extra information",
}


const endpoints = (domain) => [
"http://" + domain + "/capture",
"https://" + domain + "/capture",
]


// prepare for manifest v3
const host_permissions = endpoints('localhost')
const optional_host_permissions = endpoints('*')


// TODO make permissions literate
const permissions = [
// for keeping extension settings
"storage",

// for showing notification about successful capture or errors
"notifications",

// need to query active tab and get its url/title
"activeTab",
]


const optional_permissions = []

if (target === T.FIREFOX || v3) {
// chrome v2 doesn't support scripting api
// code has a fallback just for that
// (needed to get selected text)
permissions.push("scripting")
}


const content_security_policy = [
"script-src 'self'", // this must be specified when overriding, otherwise it complains
/// also this works, but it seems that default-src somehow shadows style-src???
// "default-src 'self'",
// "style-src 'unsafe-inline'", // FFS, otherwise <style> directives on extension's pages not working??
///

// also need to override it to eclude 'upgrade-insecure-requests' in manifest v3?
// https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Content_Security_Policy#upgrade_insecure_network_requests_in_manifest_v3
// NOTE: could be connect-src http: https: to allow all?
// but we're specifically allowing endpoints that have /capture in them
"connect-src " + endpoints('*:*').join(' '),
].join('; ')


const background = {}
if (v3) {
if (target === T.CHROME) {
// webext lint will warn about this since it's not supported in firefox yet
background['service_worker'] = 'background.js'

// this isn't supported in chrome manifest v3 (chrome warns about unsupported field)
// but without it webext lint fails
background['scripts'] = ['background.js']
} else {
background['scripts'] = ['background.js']
}
} else {
background['scripts'] = ['background.js']
background['persistent'] = false
}
background['type'] = 'module' // hmm seems like it works in firefox v2 too now??

const manifest = {
name: pkg.name + (release ? '' : ' [dev]'),
version: pkg.version,
description: pkg.description,
permissions: permissions,
commands: commands,
optional_permissions: optional_permissions,
manifest_version: v3 ? 3 : 2,
background: background,
icons: {
'128': 'img/unicorn.png'
},
options_ui: {
page: 'options.html',
open_in_tab: true,
},
}
manifest[action_name] = action

if (target === T.FIREFOX) {
// NOTE: chrome v3 works without content_security_policy??
// but in firefox it refuses to make a request even when we allow hostname permission??
manifest.content_security_policy = (v3 ? {extension_pages: content_security_policy} : content_security_policy)
}

manifest.content_scripts = [
{
"matches": ["<all_urls>"],
"js": ["detect_dark_mode.js"],
},
]

if (v3) {
if (target === T.FIREFOX) {
// firefox doesn't support optional host permissions
// note that these will still have to be granted by user (unlike in chrome)
manifest['host_permissions'] = [...host_permissions, ...optional_host_permissions]
} else {
manifest['host_permissions'] = host_permissions
manifest['optional_host_permissions'] = optional_host_permissions
}
} else {
manifest.permissions.push(...host_permissions)
manifest.optional_permissions.push(...optional_host_permissions)
}

if (v3) {
// this isn't really required in chrome, but without it, webext lint fails for chrome addon
const gecko_id = target === T.FIREFOX ? ext_id : '{00000000-0000-0000-0000-000000000000}'
manifest['browser_specific_settings'] = {
'gecko': {
'id': gecko_id,
},
}
}
return manifest
}
File renamed without changes.
25 changes: 10 additions & 15 deletions extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
"main": "dist/background.js",
"scripts": {
"test": "jest",
"build": "webpack --progress --profile",
"watch": "webpack --watch --progress",
"dev": "npm run watch",
"build": "rollup --config",
"eslint": "eslint src",
"web-ext": "web-ext",
"release:cws": "chrome-webstore-upload",
Expand All @@ -33,27 +31,24 @@
"@babel/preset-env": "^7.24.5",
"@babel/preset-typescript": "^7.24.1",
"@eslint/js": "^9.3.0",
"@rollup/plugin-commonjs": "^25.0.8",
"@rollup/plugin-node-resolve": "^15.2.3",
"@rollup/plugin-typescript": "^11.1.6",
"@types/webextension-polyfill": "^0.10.7",
"babel-loader": "^9.1.3",
"chrome-webstore-upload-cli": "^3.1.0",
"clean-webpack-plugin": "^4.0.0",
"copy-webpack-plugin": "^12.0.2",
"css-loader": "^7.1.2",
"eslint": "^8.57.0",
"globals": "^15.3.0",
"jest": "^29.5.0",
"jest-environment-jsdom": "^29.5.0",
"jest-fetch-mock": "^3.0.3",
"node-fetch": "^3.3.2",
"style-loader": "^4.0.0",
"ts-loader": "^9.5.1",
"rollup": "^4.18.0",
"rollup-plugin-copy": "^3.5.0",
"tslib": "^2.6.2",
"typescript": "^5.4.5",
"typescript-eslint": "^7.10.0",
"web-ext": "^7.11.0",
"webextension-polyfill": "^0.12.0",
"webpack": "^5.91.0",
"webpack-cli": "^5.1.4",
"webpack-dev-server": "^5.0.4",
"webpack-extension-manifest-plugin": "^0.8.0"
}
"webextension-polyfill": "^0.12.0"
},
"type": "module"
}
113 changes: 113 additions & 0 deletions extension/rollup.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import assert from 'assert'
import fs from 'fs'
const { globSync } = import('node:fs')
import path from 'path'
import { fileURLToPath } from 'url'

import typescript from '@rollup/plugin-typescript'
import { nodeResolve } from '@rollup/plugin-node-resolve'
import commonjs from '@rollup/plugin-commonjs'
import copy from 'rollup-plugin-copy'

import {generateManifest} from './generate_manifest.js'


const env = {
RELEASE: process.env.RELEASE,
PUBLISH: process.env.PUBLISH,
MANIFEST: process.env.MANIFEST,
}

const target = process.env.TARGET; assert(target)
const manifest_version = process.env.MANIFEST; assert(manifest_version)
const ext_id = process.env.EXT_ID; assert(ext_id)
const release = env.RELEASE === 'YES' // TODO use --environment=production for rollup?
const publish = env.PUBLISH === 'YES'


const thisDir = path.dirname(fileURLToPath(import.meta.url)); assert(path.isAbsolute(thisDir))
const srcDir = path.join(thisDir, 'src')
const buildDir = path.join(thisDir, 'dist', target)


// kinda annoying it's not a builtin..
function cleanOutputDir() {
return {
name: 'clean-output-dir',
buildStart(options) {
const outDir = buildDir
// we don't just want to rm -rf outputDir to respect if it's a symlink or something like that
if (!fs.existsSync(outDir)) {
return
}
fs.readdirSync(outDir).forEach(f => {
// console.debug("removing %s", f)
fs.rmSync(path.join(outDir, f), {recursive: true})
})
},
}
}


function generateManifestPlugin() {
return {
name: 'generate-manifest',
generateBundle(outputOptions, bundle) {
const manifest = generateManifest({
target: target,
version: manifest_version,
releas: release,
ext_id: ext_id,
})
const mjs = JSON.stringify(manifest, null, 2)
const outputPath = path.join(outputOptions.dir, 'manifest.json')
fs.mkdirSync(outputOptions.dir, { recursive: true })
fs.writeFileSync(outputPath, mjs, 'utf8')
}
}
}


const compile = inputs => { return {
input: inputs,
output: {
dir: buildDir,
// format: 'esm', // default??
// format: 'iife', // inlines? e.g. could use for bg page if we disable splitting..

// huh! so if I build all files in one go, it figures out the shared files properly it seems
// however it still inlines webextension stuff into one of the files? e.g. common
manualChunks: id => { // ugh, seems a bit shit?
if (id.includes('webextension-polyfill')) {
return 'webextension-polyfill' // move it in a separate chunk
}
},
},
plugins: [
cleanOutputDir(),
copy({
targets: [
{src: 'src/**/*.html', dest: buildDir},
{src: 'src/**/*.png' , dest: buildDir},
],
flatten: false,
}),
typescript({
outDir: buildDir,
noEmitOnError: true, // fail on errors
}),
commonjs(), // needed for webext polyfill
nodeResolve(),
generateManifestPlugin(),
],
}}


export default [
compile([
path.join(srcDir, 'background.ts'),
path.join(srcDir, 'options_page.ts'),
path.join(srcDir, 'popup.ts'),
path.join(srcDir, 'detect_dark_mode.ts'),
]),
]
Loading

0 comments on commit 2f78cae

Please sign in to comment.