Opened on 05/31/2016 at 09:49:36 AM

Closed on 06/02/2016 at 03:54:39 PM

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

Attachments (0)

Change History (9)

comment:1 Changed on 05/31/2016 at 11:38:03 AM by trev

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

comment:2 Changed on 05/31/2016 at 03:15:07 PM by trev

  • Description modified (diff)

comment:3 Changed on 06/01/2016 at 12:08:53 PM by abpbot

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

comment:4 Changed on 06/01/2016 at 12:09:20 PM by trev

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

comment:5 Changed on 06/01/2016 at 12:09:33 PM by trev

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

comment:6 Changed on 06/02/2016 at 03:43:27 PM 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 on 06/02/2016 at 03:46:07 PM by trev

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

comment:8 Changed on 06/02/2016 at 03:49:31 PM by abpbot

comment:9 Changed on 06/02/2016 at 03:54:39 PM by trev

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

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

Add Comment

Modify Ticket

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