Conversation
As @chancancode explained in #1120[1], the injected debug code previously did too much work in order to report a version miss. To help understand what went wrong, I'll give an abridged version of the boot-up protocol, and how the fallback logic fits in. 1. The Ember inspector application boots up inside of a devtools iframe, and injects the version of `ember_debug.js` for the latest version of the Ember inspector. 2. `ember_debug.js` to become aware of any supported version of Ember, and when that happens, it checks whether the version of Ember on the current page is supported by the current version of `ember_debug.js`. 3. If it matches, everything proceeds as usual, and `ember_debug.js` starts communicating with the inspector pane. 4. If it doesn't match, `ember_debug.js` sends a `version-mismatch` message to the inspector pane along with the current version of Ember used on the current page. The inspector pane then calculates which version of the inspector to use (currently either `0-0-0` for Ember 0.0 to Ember 2.6 or `2-7-0` for Ember 2.8 to Ember 3.2). When this happens, the inspector navigates itself to the older version of the inspector, allowing that version to bootstrap itself. Note that the inspector uses the Ember registry and loader to register its modules, so any modules for the latest version of ember_debug will *still be registered* when the older version of `ember_debug.js` is evaluated. This is not intrinsically a problem, since the calls to `define` in the new version of `ember_debug.js` will replace the old modules. Any modules that were present in the latest version but not the previous version will still be registered, but won't be used by the older code. The problem with all of that is that if any module is *required* in the process of reporting the version miss, the module's exports will remain cached in Ember's loader. Replacing the module with new source code has no effect, because the next time someone tries to require the module, it will just return the old cached exports, and the new `define` is completely ignored. This means that, depending on which code is required as part of reporting the version miss, a random Frankenstein combination of modules will be used at runtime. It should come as no surprise that this could have problems. The reason that it didn't cause any issues until recently is a coincidence: the assortment of modules in question happened to work well enough to avoid any problems that incited a user to report the bug. The solution to the problem in this commit is to completely avoid requiring any modules before the version is correctly matched. There were three modules previously required: 1. Two utility modules: `on-ready` and `version`. These modules were small so I inlined the code next to the version reporting logic. 2. The ember_debug adapter. This is a much thornier dependency, because it sucks in a whole bunch of additional dependencies and is the root cause of the current issue. I addressed this dependency by hand-coding just enough logic to create a `MessageChannel`, post it to the current window, and `postMessage` a `version-mismatch` message through the channel. This works as long as the adapter uses a `MessageChannel` to communicate the version mismatch. This is how `web-extension.js` works, which covers Chrome, Firefox, Edgium, and remote debugging using the remote debugging protocol. It does not cover other backends, but those backends are currently broken for other reasons. It doesn't substantially burden the work of making other backends work again, it simply constrains backends to use a `MessageChannel` (posted to the inspected app's window) to communicate version mismatches. It doesn't otherwise constrain any adapter functionality in `ember_debug` or the Ember Inspector. If this constraint isn't acceptable for a future backend, we could also support alternative ways of communicating the version mismatch, as long as that didn't require the version reporting code to load in the entire adapter module. [1]: #1120 (comment)
Member
Author
|
I tested this by creating a new Ember 3.3 app, copying the |
RobbieTheWagner
approved these changes
Feb 12, 2020
Member
RobbieTheWagner
left a comment
There was a problem hiding this comment.
Looks good. Thanks for tackling this! Just one question about importing and using some existing code.
locks
approved these changes
Feb 14, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1120
As @chancancode explained in #1120, the injected debug code previously did
too much work in order to report a version miss.
To help understand what went wrong, I'll give an abridged version of the
boot-up protocol, and how the fallback logic fits in.
and injects the version of
ember_debug.jsfor the latest versionof the Ember inspector.
ember_debug.jsto become aware of any supported version of Ember,and when that happens, it checks whether the version of Ember on the
current page is supported by the current version of
ember_debug.js.ember_debug.jsstarts communicating with the inspector pane.
ember_debug.jssends aversion-mismatchmessage tothe inspector pane along with the current version of Ember used on the current
page. The inspector pane then calculates which version of the inspector to use
(currently either
0-0-0for Ember 0.0 to Ember 2.6 or2-7-0for Ember 2.8to Ember 3.2). When this happens, the inspector navigates itself to the older
version of the inspector, allowing that version to bootstrap itself.
Note that the inspector uses the Ember registry and loader to register its
modules, so any modules for the latest version of ember_debug will still be
registered when the older version of
ember_debug.jsis evaluated.This is not intrinsically a problem, since the calls to
definein the newversion of
ember_debug.jswill replace the old modules. Any modules that werepresent in the latest version but not the previous version will still be
registered, but won't be used by the older code.
The problem with all of that is that if any module is required in the process
of reporting the version miss, the module's exports will remain cached in
Ember's loader. Replacing the module with new source code has no effect,
because the next time someone tries to require the module, it will just return
the old cached exports, and the new
defineis completely ignored.This means that, depending on which code is required as part of reporting the
version miss, a random Frankenstein combination of modules will be used at
runtime. It should come as no surprise that this could have problems. The
reason that it didn't cause any issues until recently is a coincidence: the
assortment of modules in question happened to work well enough to avoid any
problems that incited a user to report the bug.
The solution to the problem in this commit is to completely avoid requiring any
modules before the version is correctly matched. There were three modules
previously required:
on-readyandversion. These modules were small so Iinlined the code next to the version reporting logic.
in a whole bunch of additional dependencies and is the root cause of the current
issue. I addressed this dependency by hand-coding just enough logic to create
a
MessageChannel, post it to the current window, andpostMessageaversion-mismatchmessage through the channel.This works as long as the adapter uses a
MessageChannelto communicate theversion mismatch. This is how
web-extension.jsworks, which covers Chrome,Firefox, Edgium, and remote debugging using the remote debugging protocol.
It does not cover other backends, but those backends are currently broken for
other reasons. It doesn't substantially burden the work of making other
backends work again, it simply constrains backends to use a
MessageChannel(posted to the inspected app's window) to communicate version mismatches. It
doesn't otherwise constrain any adapter functionality in
ember_debugor theEmber Inspector. If this constraint isn't acceptable for a future backend, we
could also support alternative ways of communicating the version mismatch, as
long as that didn't require the version reporting code to load in the entire
adapter module.