922308 - Simplify JSM imports

Status: RESOLVED DUPLICATE of bug 935109

(Add-on SDK Graveyard :: General, defect, P3)

Description

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

It should be easy to import .jsm's into SDK, ideally following syntax should just do it:

require("FileUtils.jsm")
require("sessionstore/SessionStore.jsm")

In other words loader should understand `.jsm` file extension in which case loader should use `Cu.import` to load underlying module.

Comment 1

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: bug

Adds ability to load JSM files, via relative or absolute (resource) paths, and js files via resource paths:

require('./File.jsm');
require('resource://gre/modules/File.jsm');
require('resource://gre/modules/commonjs/sdk/file.js');

Comment 2

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 826229 [details]
bug

I doubt this is going to work for reasons noted in pull, could you please make a test add-on to be sure it does ?

Comment 3

[Jordan Santell [:jsantell] [@jsantell]]

Added a test addon case. 

For the CFX Module Search, it checks the local SDK working directory, even when not overloading (-o) the modules. The only reason the alias path in bootstrap.js work ('': 'resource://gre/module/commonjs') is because it matches up with the local SDK working directory ./lib. If you were to make up a file and put it in lib/sdk, and run without overloading modules, it would pass through CFX fine, but throw on runtime.

I don't see much benefit for compile time checking for module existence in this case, and we should ignore 'devtools/' alias and absolute resource URIs, especially as we are moving away from CFX. Not the a perfect scenario currently, but we have no idea from cfx what files exist outside of the SDK directory.

Added alias tests, addon tests -- need to change CFX to ignore this stuff and let it load from FF directly.

Comment 4

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 826229 [details]
bug

It would be nice to also update docs to reflect the fact that .jsm's can be loaded as `require("modules/NetUtil.jsm")` instead of currently documented
way. Also we should update our `Cu.import`'s to use this better way.

Comment 5

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Things suggested above can / should be different patches than this one.

Comment 6

[Jordan Santell [:jsantell] [@jsantell]]

Added bug 936240 for docs and bug 936239 for internal refactoring of using this

Comment 7

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/016b978994fb4c04d204484d96177a38c90c02f6
Bug 922308: Add ability to load JSMs via loader

https://github.com/mozilla/addon-sdk/commit/1d9a2fa88b8db41d155d319de833d704306e2785
Merge pull request #1277 from jsantell/loader-jsm

Fix Bug 922308: Add ability to load JSMs via loader, r=@gozala

Comment 8

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/cd2146c6d982c511cbfc6d97fb070450792cef0e
Revert "Bug 922308: Add ability to load JSMs via loader"

This reverts commit 016b978994fb4c04d204484d96177a38c90c02f6.

Comment 9

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/a50850067232d9c2e97ee7069793aa9b780e39c5
Revert "Revert "Bug 922308: Add ability to load JSMs via loader""

This reverts commit cd2146c6d982c511cbfc6d97fb070450792cef0e.

Comment 10

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/d506602d7a01f89f4b372bbd0c9e260340ebc596
Revert "Revert "Revert "Bug 922308: Add ability to load JSMs via loader"""

This reverts commit a50850067232d9c2e97ee7069793aa9b780e39c5.

Comment 11

[Jordan Santell [:jsantell] [@jsantell]]

Causing breakages:
TEST-UNEXPECTED-FAIL | layout-change/main.test compatibility | Module `self` is not found at resource://extensions.modules.test-layout-change-at-jetpack.commonjs.path/self.js
TEST-UNEXPECTED-FAIL | require/main.test 3rd party vs sdk module | Module `page-mod` is not found at resource://extensions.modules.test-require-at-jetpack.commonjs.path/page-mod.js
TEST-UNEXPECTED-FAIL | require/main.test local vs sdk module | Module `memory` is not found at resource://extensions.modules.test-require-at-jetpack.commonjs.path/memory.js
TEST-UNEXPECTED-FAIL | require/main.testSDKRequire | Module `page-worker` is not found at resource://extensions.modules.test-require-at-jetpack.commonjs.path/page-worker.js

Comment 12

[Jordan Santell [:jsantell] [@jsantell]]

Closing this, as this is going to be an option for the new nativeloader