Closed Bug 1310702 Opened 4 years ago Closed 4 years ago

add webpack-like require.context to addon-sdk require

Categories

(DevTools :: Framework, enhancement, P1)

enhancement

Tracking

(firefox52 fixed)

RESOLVED FIXED
Firefox 52
Iteration:
52.3 - Nov 14
Tracking Status
firefox52 --- fixed

People

(Reporter: tromey, Assigned: tromey)

References

Details

(Whiteboard: [devtools-html])

Attachments

(1 file)

This comes from bug 1291049.
There, we are using webpack for the inspector-in-content project.
However, devtools l10n.js uses dynamic requires which webpack doesn't like.
The solution we found to this is to introduce require.context and have l10n.js
use different require contexts for webpack.
Whiteboard: [devtools-html] → [devtools-html] [triage]
Flags: qe-verify-
Priority: -- → P2
Whiteboard: [devtools-html] [triage] → [devtools-html]
Assignee: nobody → ttromey
Status: NEW → ASSIGNED
Iteration: --- → 52.3 - Nov 7
Priority: P2 → P1
Comment on attachment 8802996 [details]
Bug 1310702 - use webpack-like require.context in devtools l10n;

https://reviewboard.mozilla.org/r/87234/#review86644

It's a little strange, but it makes sense overall.  Thanks for working on it!

::: addon-sdk/source/lib/toolkit/loader.js:800
(Diff revision 1)
>      return uri;
>    }
>  
> +  // This is like webpack's require.context.  It returns a new require
> +  // function that prepends the prefix to any requests.
> +  require.context = (prefix) => {

Nit: can drop parens for single arg

::: devtools/shared/l10n.js:11
(Diff revision 1)
>  const parsePropertiesFile = require("devtools/shared/node-properties/node-properties");
>  const { sprintf } = require("devtools/shared/sprintfjs/sprintf");
>  
>  const propertiesMap = {};
>  
> +// Some shenanigans are needed for LocalizationHelper's dynamic

Please explain in comments _why_ `require.context` is useful for Webpack with dynamic requires (like you did on IRC).  When reading the code, it seems like a no op (which it is for the DevTools loader).

::: devtools/shared/l10n.js:15
(Diff revision 1)
> +const reqShared = require.context("raw!devtools-shared/locale/",
> +                                  true, /^.*\.properties$/);
> +const reqClient = require.context("raw!devtools/locale/",
> +                                  true, /^.*\.properties$/);
> +const reqGlobal = require.context("raw!global/locale/",
> +                                  true, /^.*\.properties$/);

I don't particularly like how these module IDs for these files allows the chrome:// paths to bleed through.  I think a better design would be to use real path entries in the loader instead.  I filed bug 1312041 about this.

Anyway, that doesn't really affect the logic of what's happening here, it may just change the module IDs later.
Attachment #8802996 - Flags: review?(jryans) → review+
This version removes the last "chrome://" prefixes from uses of LocalizationHelper.
... and fix eslint.
Pushed by ttromey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ad425b5dd87b
use webpack-like require.context in devtools l10n; r=jryans
https://hg.mozilla.org/mozilla-central/rev/ad425b5dd87b
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.