If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Add a way to disable dependency checks

RESOLVED WONTFIX

Status

Add-on SDK
General
P3
normal
RESOLVED WONTFIX
5 years ago
3 years ago

People

(Reporter: anant, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

5 years ago
It seems like cfx is searching for the string "require" in order to determine what the dependencies of a module are. However, when using a library that has been optimized by Closure Tools [1], there are tons of require statements that are otherwise harmless, but are marked as unfulfilled dependencies by cfx.

Would it be possible to disable the dependency check via a command-line switch?

[1] https://developers.google.com/closure/
In theory, switching cfx to js will magically fix this.
Priority: -- → P3
Is this tool minifying the code? so that require('x'); becomes r = require;r('x'); ?

I don't think disabling this build time mapping feature will work because we do the mapping in order to determine what permissions your add-on is requesting.
Flags: needinfo?(anant)
Given the current plans to replace Py CFX with CFX.js, we're not going to fix this on the python side. Cancelling the needinfo to clean up the dashboard a tiny bit.
Flags: needinfo?(anant)
I think the JS-based dependency scanner will have exactly the same problems as the python-based one.

I've used Anant's code (Firebase) a little bit. If I remember correctly, the Closure code was defining a require() function and then calling it several times for internal purposes, completely unrelated to the require() of node.js/requirejs/jetpack. This definition was inside a big function itself, so I think it shadows the require() provided by jetpack at the top-level scope.

We've talked about a more sophisticated dependency scanner which uses the same lexer/parser as spidermonkey itself and works on the AST instead of using a bunch of regexps, which might be easier with a JS-based scanner if that environment provides access to its own parser. That would at least make it easier to ignore require() calls inside quotes and block comments. Detecting a shadowed version of require() would go beyond the call of duty, I think.. that sounds pretty hard.

(there is another issue, in which several require() statements are really intended as require(), but they're if/then/else'd off unless the code detects it's running inside node.js or require.js . When it's in a regular web-context, it relies on DOM properties instead, for stuff like WebSockets. These cause a similar problem unless the specific name being imported can be the same for jetpack as for node.js/requirejs.)

One other solution I suggested to Anant was to add a build step that takes the output of Closure, search-and-replaces s/require/FOO/, then prepend the jetpack-specific require() statements. Basically anything to convince Closure to use some other name for its internal "require" functionality.

Or we could add some sort of magic comment to the jetpack syntax, like:

// JETPACK-DEPENDS: sdk/page-mod, sdk/tabs, ./mystuff

and teach the dependency scanner that when it sees that line, it ignores the rest of the file and instead pretends that it just saw require("sdk/page-mod") etc. Pretty straightforward, pretty reliable, pretty easy to explain.
(In reply to Brian Warner [:warner :bwarner] from comment #4)
> I think the JS-based dependency scanner will have exactly the same problems
> as the python-based one.

+1

I'm wondering one thing: Are those modules optimized by closure regular commonjs module that you load using jetpack require() statement?
If not, these files can be moved to data/ older and avoid being analysed by the dependency parser.
Sorry we won't be releasing any new versions of cfx, jpm is the replacement https://www.npmjs.com/package/jpm

Note: JPM doesn't do any require/dependency checks.
Blocks: 840023
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.