Jetpack 1.12 - toolkit/loader, globals are broken

RESOLVED WORKSFORME

Status

Add-on SDK
General
P3
normal
RESOLVED WORKSFORME
6 years ago
3 years ago

People

(Reporter: simon.lachinger, Assigned: irakli)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.11 (KHTML, like Gecko) Sabayon Chrome/23.0.1271.91 Safari/537.11

Steps to reproduce:

I tried to reuse javascript code spread over several files from the pre-jetpack addon implementation which share a common namespace:
com.mycompany = { .. }

According to the docs, using the "toolkit/loader" module and the "com" object as a global, this should work similar to this:

/* main.js */
var com = {};
var loader = require('toolkit/loader');
var packaging = require('@loader/options');
var custom_options = loader.override(
    JSON.parse(JSON.stringify(packaging)),
    { globals: { 'com': com,
                 console: console,
                 module: module,
                 exports: exports,
                 require: require,
                 dump: dump
               },
    });
// now create the custom loader
var custom_loader = loader.Loader(custom_options);
var custom_require = loader.Require( custom_loader, module );
// load the files required by the addon
custom_require('path/to/code_using_com.js');
custom_require('path/to/another_code_using_com.js');
custom_require('path/to/more_code_using_com.js');



Actual results:

The 'com' object was not passed on to the code loaded with custom_require(). Debugging showed that it gets lost during the creation of the sandbox:

This snipped is taken from the load() function in sdk/lib/toolkit/loader.js:

  let sandbox = sandboxes[module.uri] = Sandbox({
    name: module.uri,
    // Get an existing module sandbox, if any, so we can reuse its compartment
    // when creating the new one to reduce memory consumption.
    sandbox: sandboxes[keys(sandboxes).shift()],
    // We expose set of properties defined by `CommonJS` specification via
    // prototype of the sandbox. Also globals are deeper in the prototype
    // chain so that each module has access to them as well.
    prototype: create(globals, descriptor({
      require: require,
      module: module,
      exports: module.exports
    })),
    wantXrays: false
  });

The object.create() method used hides the 'com' object in the prototype. Which it shouldn't actually do, however if I dump it it is gone:
dump(JSON.stringify(create(globals, descriptor({
      require: require,
      module: module,
      exports: module.exports
    }));

Also, if I dump 'com' in one of the loaded files it is 'undefined'.


Expected results:

Globals should work as described here:
https://addons.mozilla.org/en-US/developers/docs/sdk/latest/modules/toolkit/loader.html

The "com" object in the sandbox used for loading of the files should be a reference to the one defined in main.js

This actually worked with addon-sdk-1.8 but wasn't a documented feature at the time. Now it is documented, but fails.

Updated

6 years ago
Priority: -- → P3
Assignee: nobody → rFobic
Hey Irakli, you've had this bug assigned to you for quite some time, are you planning to work on it?  and how important is it?
Flags: needinfo?(rFobic)
Created attachment 8555581 [details] [review]
Test for the described issues that is passing.
Flags: needinfo?(rFobic)
Attachment #8555581 - Flags: review?(evold)
simon it's being a while & I don't know if this bug was resolved since you've tried it or if there was something wrong with your code. Anyhow I created test case (attached to the bug) which does passes with a current master. Although it's not a exactly the code you've written since:

1. JSON.parse(JSON.stringify(packaging)) is not supported since there is no guarantees that packaging is serializable.
2. `module`, `require` and `exports` objects are not supposed to be shared across modules, so putting them into global is not valid use of the API. That being said I highly doubt that was an issue here.

As of the following comment:

> The object.create() method used hides the 'com' object in the prototype. Which it shouldn't actually do, however if I dump it it is gone:

Shared globals are intentionally put into prototype chain instead of copying it's properties onto every single sandbox that way changes to them are also shared across all modules. Either way that can't be an issues.

I suggest we close it as works for me after test case lands.
Attachment #8555581 - Flags: review?(evold) → review+

Comment 4

3 years ago
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/1c6a6c85e3d908027c15a3e848edcc3303cae1aa
Create a test case to cover scenario from the bug 827792.

https://github.com/mozilla/addon-sdk/commit/038e7678b76d11510779d98a2b785272da585515
Merge pull request #1845 from Gozala/bug/loader-globals@827792

Bug 827792 - Create a test case to cover scenario from the bug r=erikvold
Status: UNCONFIRMED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.