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)


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:

  1. 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.
  2. 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

Blocked by #5751 since we need to remove buildtools/lib to avoid webpack mistakenly resolving the prefs module from there.

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 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.

Firefox 50 / 57 / Windows 10

Chrome 49 / 62 / Windows 10
Opera 36 / 48 / Windows 10

Add Comment

Modify Ticket

Change Properties
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none).
Note: See TracTickets for help on using tickets.