1310774 - Make JSON viewer use shared search-box component

Status: NEW

(DevTools :: JSON Viewer, defect, P3)

Description

[Tim Nguyen :ntim]

Use devtools/client/shared/components/search-box.js instead of devtools/client/jsonview/components/search-box.js

Comment 1

[Dalimil Hajek [:dalimil]]

When I import the search-box component (const SearchBox = require("devtools/client/shared/components/search-box");), I get the error: Error: Module name "devtools/client/shared/key-shortcuts" has not been loaded yet for context: _. Use require([]) http://requirejs.org/docs/errors.html#notloaded

key-shortcuts is part of the search-box component, but wrapping the search-box module in define(function (require, exports, module) {}  doesn't solve the issue (it only generates similar errors for other modules further in the dependency chain)...

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@Dalimil: can you please attach your patch so, I can try it?

Honzas

Comment 3

[Dalimil Hajek [:dalimil]]

Attachment: Bug1310774.patch - rev1

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks for the patch Dalimil.

So yes, the module "devtools/client/shared/key-shortcuts" (+ all the other modules it requires) are not ready for being loaded through RequireJS. I am not sure if wrapping all the modules (key-shortcuts, event-emitter, keycodes) within `define()` function is the right way to go. Plus also, key-shortcuts.js is requiring an extra "Services" module.

There are also following options:

1) Time to use webpack.config for JSON Viewer? Does it solve the issue entirely? What about the "Services" module?
2) Do not explicitly require KeyShortcuts in the search-box component, but pass it in. We might have a helper that does that.

Brian, any tips?

Honza

Comment 5

[Brian Grinstead [:bgrins]]

Sorry for missing this request.  If we can work around this with (2) that seems ideal, since it'd be less work and we won't need to check in a bundle / load duplicate modules

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8813444 [details] [diff] [review]
Bug1310774.patch - rev1

Just removing feedback request (already answered)

Honza

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

No activity, moving to the backlog (P3)
Honza