-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🏗 Share dependencies in bento.js
#36432
Conversation
88908f1
to
a8ba389
Compare
Hey @erwinmombay, @jridgewell! These files were changed:
|
0669e0f
to
fde7238
Compare
84708c9
to
46d9798
Compare
46d9798
to
bd250e5
Compare
bab442a
to
b628cc1
Compare
faba866
to
d999f99
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we ever discuss why we are pursuing this approach vs. the naive way?
My impression is that naively we could manually add all of the imports into bento.js like so:
// bento.js
import {PreactBaseElement} from '#preact/base-element';
window.BENTO['PreactBaseElement'] = PreactBaseElement;
// amp-fit-text/1.0/base-element.js
class FitText extends window.BENTO.PreactBaseElement {
...
}
I think what you've written in this PR is a babel transform which automatically performs the same translation right? That means we don't need to modify the source and can easily update which modules are included in bento.js over time.
d999f99
to
e1a903e
Compare
|
6e00946
to
e7eeb8c
Compare
@alanorozco: are there any blockers for this PR? |
Partial for #36421
bento-*.js
files now use shared modules from a globalBENTO
.Shared modules are listed on
shared-bento-symbols.js
.We generate
bento.js
to import and provide the dependencies based on this list. We also generatebento-shared.js
to use the dependencies from the global.bento-*.js
binaries usemodule-resolver
so that the listed imports are replaced withbento-shared.js
.Also adds the flag
--bento_runtime_only
.