Specifying "dependencies" in package.json should be additive, not destructive.

RESOLVED WONTFIX

Status

Add-on SDK
General
RESOLVED WONTFIX
6 years ago
5 years ago

People

(Reporter: KWierso, Assigned: irakli)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

6 years ago
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
(Reporter)

Comment 1

6 years ago
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.
Whiteboard: [triage:followup]
Brian: you've been the most active in specifying how modules are resolved by the loader.  What say you?
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.
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?
OS: Windows 7 → All
Priority: -- → P2
Hardware: x86 → All
Whiteboard: [triage:followup]
Target Milestone: --- → Future
Assignee: nobody → warner-bugzilla
(Reporter)

Comment 5

6 years ago
Any thoughts on this, Brian?
(Reporter)

Updated

6 years ago
Whiteboard: [triage:followup]
(Reporter)

Comment 6

6 years ago
(Pushing all open bugs to the --- milestone for the new triage system)
Target Milestone: Future → ---
Making sure we look at this in the next triage session
Priority: P2 → --
Assignee: warner-bugzilla → rFobic
In SDK 2.0 there will be no packages, so this is less relevant.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
(Reporter)

Updated

5 years ago
Whiteboard: [triage:followup]
You need to log in before you can comment on or make changes to this bug.