825791 - Add a way to disable dependency checks

Status: RESOLVED WONTFIX

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

Description

[Anant Narayanan [:anant]]

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/

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

In theory, switching cfx to js will magically fix this.

Comment 2

[Erik Vold [:erikvold] (please needinfo? me)]

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.

Comment 3

[Jeff Griffiths (:canuckistani) (:⚡︎)]

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.

Comment 4

[Brian Warner [:warner :bwarner]]

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.

Comment 5

[Alexandre Poirot [:ochameau]]

(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.

Comment 6

[Erik Vold [:erikvold] (please needinfo? me)]

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.