Opened 7 months ago

Closed 4 days ago

Last modified 3 days ago

#5080 closed change (fixed)

Switch module system to webpack, use modules for our content scripts

Reported by: kzar Assignee:
Priority: P2 Milestone: Adblock-Plus-for-Chrome-Opera-Firefox-next
Module: Platform Keywords:
Cc: sebastian Blocked By: #4864, #5028, #5077, #5089, #5535
Blocking: #5084 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29577555/

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:

  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.

Change History (22)

comment:1 Changed 7 months ago by kzar

  • Blocked By 4864 added; 4868 removed
  • Description modified (diff)

comment:2 Changed 7 months ago by sebastian

  • Blocking 5084 added

comment:3 Changed 7 months ago by kzar

  • Blocked By 5089 added

comment:4 Changed 7 months ago by kzar

  • Blocked By 5079 removed

comment:5 Changed 7 months ago by kzar

  • Owner kzar deleted

comment:6 Changed 2 months ago by kzar

  • Blocking 3138 added

comment:7 Changed 2 months ago by kzar

  • Blocked By 5535 added

comment:8 Changed 5 weeks ago by kzar

  • Blocked By 5593 added

comment:9 Changed 5 weeks ago by kzar

  • Blocked By 5708 added

comment:10 Changed 5 weeks ago by kzar

  • Blocked By 5593 removed

comment:11 Changed 5 weeks ago by kzar

  • Blocked By 5708 removed

comment:12 Changed 5 weeks ago by kzar

  • Blocked By 5593 added

comment:13 Changed 4 weeks ago by kzar

  • Blocked By 5750 added

comment:14 Changed 4 weeks ago 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 4 weeks ago by sebastian

  • Blocked By 5751 removed

comment:16 Changed 13 days ago by kzar

  • Blocked By 5593 removed

comment:17 Changed 12 days ago 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 7 days ago 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 7 days ago by kzar

  • Description modified (diff)

comment:20 Changed 4 days ago by abpbot

A commit referencing this issue has landed:
Issue 5080 - Start using our new webpack build system

comment:21 Changed 4 days ago 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 3 days ago by kzar

  • Blocking 3138 removed
Note: See TracTickets for help on using tickets.