Hot reload of the toolbox breaks the inspector with "Error: Module `acorn/acorn` is not found at resource://devtools/acorn/acorn.js"

RESOLVED FIXED in Firefox 52

Status

RESOLVED FIXED
2 years ago
4 months ago

People

(Reporter: pbro, Assigned: ochameau)

Tracking

unspecified
Firefox 52

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
I've seen this yesterday when trying to hot reload the toolbox as described on https://developer.mozilla.org/en-US/docs/Tools/Contributing/Contribute_on_nightly and Daisuke has also run into this problem, see https://bugzilla.mozilla.org/show_bug.cgi?id=1210795#c43 .

I think the STR are quite simple:

- follow the steps at https://developer.mozilla.org/en-US/docs/Tools/Contributing/Contribute_on_nightly
- when you're on the inspector panel, try to hot reload

The error shown in the browser console is:
JavaScript error: resource://devtools/shared/Loader.jsm, line 74: Error: Module `acorn/acorn` is not found at resource://devtools/acorn/acorn.js
(Reporter)

Comment 1

2 years ago
Alex, any idea what could have broken lately?
Flags: needinfo?(poirot.alex)
Assignee: nobody → poirot.alex
Flags: needinfo?(poirot.alex)
Created attachment 8791925 [details] [diff] [review]
Package acorn lib with same path as sources to fix hot reload.

Weird it breaks only now, may be acorn wasn't used before in a common codepath.
acorn was still package in omni.jar differently than the sources are organized on fs.
For the hot reload, we have to ensure not doing any magic during packaging and
put the files in the jar file with the exact same tree of files.
Attachment #8791925 - Flags: review?(pbrosset)
(Reporter)

Comment 3

2 years ago
Comment on attachment 8791925 [details] [diff] [review]
Package acorn lib with same path as sources to fix hot reload.

Review of attachment 8791925 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/shared/Loader.jsm
@@ +48,2 @@
>        // ⚠ DISCUSSION ON DEV-DEVELOPER-TOOLS REQUIRED BEFORE MODIFYING ⚠
> +      "acorn/util/walk": "resource://devtools/shared/acorn/walk.js",

We have big warning signs all over this part of the file :)
⚠ DISCUSSION ON DEV-DEVELOPER-TOOLS REQUIRED BEFORE MODIFYING ⚠

Can you explain to me what is the impact of changing these paths? Is this going to break addons for example?
Attachment #8791925 - Flags: review?(pbrosset)
:jryans put these comments in order to prevent adding any new one.

Ideally we would get rid of these acorn shortcuts and just use the full path to require them:
  require("devtools/shared/acorn/walk.js")
  require("devtools/shared/acorn/acorn.js")
  ...

But I just wanted to be conservative here and not require to change the require path everywhere.
When I introduced the hot reload addon I converted most of these shortcuts to use absolute require path, like gcli, pretty-fast, ...

With such conservative change, it won't break addon as it doesn't require change of require path.
But I'm not sure any addon require acorn directly?
(Reporter)

Comment 5

2 years ago
(In reply to Alexandre Poirot [:ochameau] from comment #4)
> :jryans put these comments in order to prevent adding any new one.
> 
> Ideally we would get rid of these acorn shortcuts and just use the full path
> to require them:
>   require("devtools/shared/acorn/walk.js")
>   require("devtools/shared/acorn/acorn.js")
>   ...
> 
> But I just wanted to be conservative here and not require to change the
> require path everywhere.
> When I introduced the hot reload addon I converted most of these shortcuts
> to use absolute require path, like gcli, pretty-fast, ...
> 
> With such conservative change, it won't break addon as it doesn't require
> change of require path.
Thanks for the explanation.

> But I'm not sure any addon require acorn directly?
Yeah, I agree, I just wanted to double-check what would be the impact here.
I will R+.
(Reporter)

Updated

2 years ago
Attachment #8791925 - Flags: review+
Created attachment 8792480 [details] [diff] [review]
Package acorn lib with same path as sources to fix hot reload.

Forgot to rewrite the absolute url from: devtools/server/actors/pretty-print-worker.js
Attachment #8792480 - Flags: review+
Attachment #8791925 - Attachment is obsolete: true
https://hg.mozilla.org/integration/fx-team/rev/de1a8aef2e8dea90c6ca09a7cb16edef83d0b27f
Bug 1303268 - Package acorn lib with same path as sources to fix hot reload. r=pbro

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/de1a8aef2e8d
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52

Updated

4 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.