When using loader outside of the SDK environment, there are a few modules that use the exports of toolkit/loader within themselves. Example: sdk/tabs includes console/traceback which uses `parseStack` and `sourceURI` from toolkit/loader. These utilities clutter up toolkit/loader, add a larger barrier of entry to using the loader outside of the SDK (in this case, 'Components' is used in toolkit/loader and cuddlefish loader has a work around, but this reduces our pile of loader work arounds), and introduces bad smells from circular dependency and don't even really belong in a loader module.

For example, in bootstrap.js, we have to make an exception for modules including the loader itself: `loaderOptions.modules['toolkit/loader'] = loaderSandbox.exports;` That's extra work and scaffolding needed to use Loader with any modules using the utilities in toolkit/loader

While bug 964452 fixes the circular dependency issue outside of the SDK environment, we should still remove some of the utilites in toolkit/loader to its own file so we don't have a 923 line file. Should have lib/toolkit/utils.js that can be loaded as a JSM for toolkit/loader, containing loading utilities

This pulls out loader functions into several other files, making loader logic focused on loading. All these are tested via loader's current tests, and what was previously exported is still exported to not break others using all the exports on the loader. Maybe in the future we can have individual unit tests for these functions as well. There are still some files requiring toolkit/loader, but those are all ones that we instrument ourselves (cuddlefish, addon/runner, test/loader), so I think we'll be okay there. Just things like sdk/selection using console/traceback and ultimately toolkit/loader should be avoided. *console-utils.js -- This is what other files (test/harness, console/traceback) are using for parsing stack traces, so it can be loaded again as a common JS module. (The following are JSMs for ease of loading, and they use Components which is a cause of some of these recursive problems -- they should not be directly loaded via the SDK) * parser.jsm -- This is the browserify+Reflect parsing methods we use for generating maps for native loaders * node-resolve.jsm -- This is the `nodeResolve` method with all of its helpers. It's pretty large and contained within one method, so this is a good move i think. * utils.jsm -- These are all other utilities used throughout toolkit/loader, and the other utils in this PR (node-resolve, parser).

Comment on attachment 8368355 [details] GH PR 1369 Looks good to me

Reverted changes back so only stack-utils are pulled out, and this stack-utils are what is referenced throughout the SDK where necessary, r+ by gozala with these changes

Commits pushed to master at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/160be1d330cba70b13e0121f90ec078f9ac2b3a7 Bug 961194 - Pull out stack utils in loader to prevent circular dependencies in SDK https://github.com/mozilla/addon-sdk/commit/aaf3c9061f1b8cc97deca7417937f029b9d1ea75 Merge pull request #1369 from jsantell/961194-modularize-loader-utils fix Bug 961194 Modularize toolkit/loader, r=@erikvold, @gozala

This broke the fx-team tree tests, see bug 972925

I had to revert the patch here https://github.com/mozilla/addon-sdk/commit/036238d8bd1bfbbbc2a44467be646711e3aeed19

Noted, was looking into this and a bit stumped. Will try to fix this up for a future release

Unassigning myself from bugs I haven't gotten around to

I wanted to look at this but I won't be able to.