Opened on 03/31/2017 at 08:57:03 AM
Closed on 10/18/2017 at 11:27:17 AM
Last modified on 11/06/2017 at 02:24:18 PM
#5080 closed change (fixed)
Switch module system to webpack, use modules for our content scripts
| Reported by: | kzar | Assignee: | |
|---|---|---|---|
| Priority: | P2 | Milestone: | Adblock-Plus-3.0.2-for-Firefox |
| Module: | Platform | Keywords: | |
| Cc: | sebastian | Blocked By: | #4864, #5028, #5077, #5089, #5535 |
| Blocking: | #5084 | Platform: | Unknown / Cross platform |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): | |||
Description (last modified by kzar)
Background
With the ESLint changes in #4864 we needed to add some /* globals ... */ comments in order to get linting to pass. We now want to reduce those by breaking the content scripts into proper modules.
Unfortunately our bespoke module system had quite a few limitations, switching our content scripts to use it wasn't easy. So we've replaced it with webpack.
What to change
- Update the buildtools dependency to hg:7e896c368056 git:960ff2e (no other changes will be included).
- Replace the [convert_js] section of our metadata files with [bundles]. Remove autoload, module and injectinfomodule options. Ensure only the modules which need to be autoloaded are listed, the others will be resolved by webpack automatically.
- Bundle the relevant content scripts, exporting the globals used by the other content sctipts and replacing global comments with proper require statements. (Also require the things exported by lib/elemHideEmulation.js see #5079.)
Hints for testers
This is a pretty major behind the scenes change, with hopefully only the following noticeable differences:
- The Sources pane of the developer tools for the extension's background page should have a webpack:// section which lists all the source code files as they were pre-bundling.
- If you attempt to use the require function in the Console pane of the background page's developer tools it will no longer work.
It's hard to say what needs to be tested, in theory anything could have broken. I think at least all major functionality needs to be briefly tested for all supported browsers, ideally both a new and old supported version of each.
Attachments (0)
Change History (23)
comment:1 Changed on 03/31/2017 at 11:49:02 AM by kzar
- Blocked By 4864 added; 4868 removed
- Description modified (diff)
comment:2 Changed on 04/03/2017 at 06:04:29 AM by sebastian
- Blocking 5084 added
comment:3 Changed on 04/03/2017 at 07:52:13 AM by kzar
- Blocked By 5089 added
comment:4 Changed on 04/03/2017 at 07:52:35 AM by kzar
- Blocked By 5079 removed
comment:5 Changed on 04/03/2017 at 08:11:41 AM by kzar
- Owner kzar deleted
comment:6 Changed on 08/09/2017 at 03:32:22 PM by kzar
- Blocking 3138 added
comment:7 Changed on 08/18/2017 at 11:42:26 AM by kzar
- Blocked By 5535 added
comment:8 Changed on 09/18/2017 at 11:10:25 AM by kzar
- Blocked By 5593 added
comment:9 Changed on 09/18/2017 at 01:28:11 PM by kzar
- Blocked By 5708 added
comment:10 Changed on 09/18/2017 at 01:29:27 PM by kzar
- Blocked By 5593 removed
comment:11 Changed on 09/19/2017 at 01:44:14 PM by kzar
- Blocked By 5708 removed
comment:12 Changed on 09/19/2017 at 01:45:15 PM by kzar
- Blocked By 5593 added
comment:13 Changed on 09/22/2017 at 10:23:06 AM by kzar
- Blocked By 5750 added
comment:14 Changed on 09/23/2017 at 09:42:26 AM by kzar
- Blocked By 5751 added
comment:15 Changed on 09/23/2017 at 06:28:39 PM by sebastian
- Blocked By 5751 removed
comment:16 Changed on 10/09/2017 at 05:02:09 PM by kzar
- Blocked By 5593 removed
comment:17 Changed on 10/10/2017 at 10:17:26 AM by kzar
- Blocked By 5750 removed
(The optional requires have already been removed as part of #5750 and so that issue is no longer blocking us here, despite being unfinished.)
comment:18 Changed on 10/15/2017 at 01:31:02 PM by kzar
- Blocked By 5028 added
- Description modified (diff)
- Review URL(s) modified (diff)
- Status changed from new to reviewing
- Summary changed from Start using modules for the content scripts to Switch module system to webpack, use modules for our content scripts
(Marking this as being blocked by #5028 since I'm hoping the buildtools dependency will be updated to the previous commit there, and the copy of ensure_depdencies.py updated along with it. Otherwise the package-lock.json Node.js file could re-appear for people and cause problems.)
comment:19 Changed on 10/15/2017 at 01:57:41 PM by kzar
- Description modified (diff)
comment:20 Changed on 10/18/2017 at 11:05:27 AM by abpbot
A commit referencing this issue has landed:
Issue 5080 - Start using our new webpack build system
comment:21 Changed on 10/18/2017 at 11:27:17 AM by kzar
- Description modified (diff)
- Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
- Ready set
- Resolution set to fixed
- Status changed from reviewing to closed
comment:22 Changed on 10/19/2017 at 10:27:17 AM by kzar
- Blocking 3138 removed
comment:23 Changed on 11/06/2017 at 02:24:18 PM by Ross
- Milestone changed from Adblock-Plus-3.0-for-Chrome-Opera-Firefox to Adblock-Plus-for-Chrome-Opera-Firefox-next
- Tester changed from Unknown to Ross
- Verified working set
Done. Did not find any regressions caused by this change.
ABP 3.0.0.1911beta
Firefox 50 / 57 / Windows 10
ABP 1.13.4.1903
Chrome 49 / 62 / Windows 10
Opera 36 / 48 / Windows 10
Blocked by #5751 since we need to remove buildtools/lib to avoid webpack mistakenly resolving the prefs module from there.