Last Comment Bug 663480 - Specifying "dependencies" in package.json should be additive, not destructive.
: Specifying "dependencies" in package.json should be additive, not destructive.
Status: RESOLVED WONTFIX
:
Product: Add-on SDK
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Irakli Gozalishvili [:irakli] [:gozala] [@gozala]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-10 11:33 PDT by Wes Kocher (:KWierso)
Modified: 2012-07-24 01:17 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments

Description Wes Kocher (:KWierso) 2011-06-10 11:33:22 PDT
From dietrich on IRC:
<dietrich>	packages dir in add-on root should be additive tothe core
	not replace the core
	(in terms of absolute dependencies)
	i guess it's not about the packages dir though
	<peregrino>	dietrich: yes, but I think that is something not yet fully documented
<dietrich>	it's the fact that specifying anything in the dependencies field in package.json blows away the default dependendencies
Comment 1 Wes Kocher (:KWierso) 2011-06-10 11:36:48 PDT
I could see how specifying the packages used to be useful to get a smaller xpi file when you're done, since you would be able to completely bypass the SDK and only use the packages that you specify.

But now that cfx excludes packages that aren't specified, specifying packages dir would be more useful as an additive change.
Comment 2 Myk Melez [:myk] [@mykmelez] 2011-06-13 14:08:05 PDT
Brian: you've been the most active in specifying how modules are resolved by the loader.  What say you?
Comment 3 Brian Warner [:warner :bwarner] 2011-06-14 16:56:08 PDT
Hmm. I agree that additive would be easier to use most of the time.

The use of the ".dependencies" property is a bit weird right now, and needs
cleanup. CommonJS defines it to be a dictionary (not a list), with version
numbers as values, but is kind of thin on how it's really supposed to be
used. In NPM (which seems to change monthly), it appears to be merely
advisory, informing a package-installation process about which dependent
packages should be downloaded and installed for future use, rather than
anything which affects the search process.

Jetpack has traditionally used it to populate a total-ordered list of
packages in which the linker will search for modules. After the linker-search
changes in b5/1.0, it is also used to control the search path for individual
packages. static-files/md/dev-guide/addon-development/module-search.md should
describe the behavior. The "destructive" behavior show up because this code
(manifest.py line 500) switches on the presence of an individual package's
.dependencies property to decide which list to search, and the usual
"addon-kit" only shows up in the total-ordered list (as a default value to
the --extra-packages argument).

I wasn't paying too much attention to the behavior of .dependencies, but I
was ok with it being destructive because I figured that otherwise there'd be
no way to *remove* addon-kit from the search list. But yeah, it's kind of
annoying and surprising. And you could probably do --extra-packages="" or
something to remove addon-kit.

One thing that needs to be balanced is clarity of the rules versus making it
easy to be brief in your top-level addon package. The .dependencies for the
top-level package defaults to ["addon-kit"] so that authors can do
require("panel") instead of require("addon-kit/panel"), although arguably the
latter is better (less ambiguous, less magical).

Anyways, yeah, I'm not yet sure what to do. I'd kind of like to get rid of
.dependencies altogether.
Comment 4 Myk Melez [:myk] [@mykmelez] 2011-06-15 12:09:18 PDT
It sounds like we at least need to figure out what to do here, even if we ultimately decide not to do anything.  So P2 on deciding what to do.

Brian: can you take on the task of figuring this out?
Comment 5 Wes Kocher (:KWierso) 2011-08-08 13:36:34 PDT
Any thoughts on this, Brian?
Comment 6 Wes Kocher (:KWierso) 2011-09-08 12:10:14 PDT
(Pushing all open bugs to the --- milestone for the new triage system)
Comment 7 Dave Townsend [:mossop] 2012-04-12 12:00:49 PDT
Making sure we look at this in the next triage session
Comment 8 Irakli Gozalishvili [:irakli] [:gozala] [@gozala] 2012-05-04 14:47:40 PDT
In SDK 2.0 there will be no packages, so this is less relevant.

Note You need to log in before you can comment on or make changes to this bug.