馃毃 Error: Doc factory failed: amp-mustache: Cannot read property 'amp-mustache' of undefined #58
Comments
|
/cc @dvoytenko This seems like it may be directly related to ampproject/amphtml#32853 |
|
/cc reviewers @calebcordry @jridgewell for visibility since it looks like Dima is OOO atm |
|
A duplicate error report was linked to this issue (link) |
|
A duplicate error report was linked to this issue (link) |
|
Investigating now. It only affects nightly, so if I can land a fix we should be ok. |
|
This is a really fun cross-module private name mangling issue. We seem to have multiple copies of the |
|
Reverted the PR, so this should disappear. But I'm going to leave this open while I try to fix the real underlying issues so we can rollfoward. |
|
Ugh. I saw that private |
jridgewell
added a commit
to jridgewell/amphtml
that referenced
this issue
Feb 26, 2021
There are several methods of sharing services with FIE, but the 2 main ones are: 1. FIE installs its own factory 2. FIE copies the parent's factory instance ampproject/error-reporting#58 is caused because the FIE is using its own factory instance (which means it has its own `Templates` class code, with private names mangled, in each binary that installs the FIE). But each FIE then uses code shared with v0.js to install a amp-mustache instance. This means we have code compiled in v0.js trying to directly interact with a class that's provided by, eg, amp-ad-custom (a separate binary!). This is never good. What's really needed here is a 3rd way to share services with FIE: 3. FIE copies the parent's **factory** FIE can then instantiate its own instance of that factory, which will have its own instance data. And because v0.js we'll reuse the class code already in v0.js, we won't have an issue with private mangling. And, we'll make all of these FIE installers smaller!
jridgewell
added a commit
to jridgewell/amphtml
that referenced
this issue
Feb 26, 2021
There are several methods of sharing services with FIE, but the 2 main ones are: 1. FIE installs its own factory 2. FIE copies the parent's factory instance ampproject/error-reporting#58 is caused because the FIE is using its own factory instance (which means it has its own `Templates` class code, with private names mangled, in each binary that installs the FIE). But each FIE then uses code shared with v0.js to install a amp-mustache instance. This means we have code compiled in v0.js trying to directly interact with a class that's provided by, eg, amp-ad-custom (a separate binary!). This is never good. What's really needed here is a 3rd way to share services with FIE: 3. FIE copies the parent's **factory** FIE can then instantiate its own instance of that factory, which will have its own instance data. And because v0.js we'll reuse the class code already in v0.js, we won't have an issue with private mangling. And, we'll make all of these FIE installers smaller!
jridgewell
added a commit
to ampproject/amphtml
that referenced
this issue
Feb 27, 2021
* Allow Service Factories to be adopted by FIE There are several methods of sharing services with FIE, but the 2 main ones are: 1. FIE installs its own factory 2. FIE copies the parent's factory instance ampproject/error-reporting#58 is caused because the FIE is using its own factory instance (which means it has its own `Templates` class code, with private names mangled, in each binary that installs the FIE). But each FIE then uses code shared with v0.js to install a amp-mustache instance. This means we have code compiled in v0.js trying to directly interact with a class that's provided by, eg, amp-ad-custom (a separate binary!). This is never good. What's really needed here is a 3rd way to share services with FIE: 3. FIE copies the parent's **factory** FIE can then instantiate its own instance of that factory, which will have its own instance data. And because v0.js we'll reuse the class code already in v0.js, we won't have an issue with private mangling. And, we'll make all of these FIE installers smaller! * Allow disposal of a shared factory's instances * Fix merge conflict * Tests * Update test files * Update shared instance test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Details
Error report: link
First seen: Feb 25, 2021
Frequency: ~ 452/day
Stacktrace
Notes
@dvoytenkomodifiedsrc/service/extensions-impl.js:339-352in #32853 (Feb 24, 2021)@dvoytenkomodifiedsrc/service/extensions-impl.js:490-495in #10992 (Aug 28, 2017)@renovate-botmodifiedsrc/service/extensions-impl.js:489-489in #27350 (Mar 30, 2020)Seen in:
Possible assignees:
@dvoytenko/cc @ampproject/release-on-duty
The text was updated successfully, but these errors were encountered: