Opened 3 years ago

Closed 3 years ago

#4090 closed change (fixed)

Make require() load modules lazily

Reported by: trev Assignee: trev
Priority: P3 Milestone: Adblock-Plus-1.12.1-for-Chrome-Opera-Safari
Module: Platform Keywords:
Cc: sebastian, kzar Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29345407/
https://codereview.adblockplus.org/29345518/

Description (last modified by trev)

Background

Currently, all modules will load immediately in order to generate their scopes. This makes it necessary to specify them in a very specific order in the convert_js section of metadata.common, otherwise require could be called for a module before its scope is defined.

What to change

With #4088 the module scopes won't be executed immediately. Our require() implementation should then call the scope when called for the first time for a module.

The dependency on buildtools repository needs to be updated in order to import #4088. This will also import three Noissue changes - these are only about coding style and have no impact on the build.

Change History (9)

comment:1 Changed 3 years ago by trev

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:2 Changed 3 years ago by trev

  • Description modified (diff)

comment:3 Changed 3 years ago by abpbot

A commit referencing this issue has landed:
Issue 4090 - Make require() load modules lazily

comment:4 Changed 3 years ago by trev

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:5 Changed 3 years ago by trev

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next

comment:6 Changed 3 years ago by trev

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening, this caused issues in older Chrome versions - lib/compat.js isn't allowed to use new ECMAScript syntax (let statement in this case).

comment:7 Changed 3 years ago by trev

  • Review URL(s) modified (diff)
  • Status changed from reopened to reviewing

comment:9 Changed 3 years ago by trev

  • Resolution set to fixed
  • Status changed from reviewing to closed

I verified that Adblock Plus works correctly again on Chrome 31 now.

Note: See TracTickets for help on using tickets.