Skip to content
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

Experiment turning on splitted vendor integration JS #32763

Merged
merged 1 commit into from
Mar 22, 2021

Conversation

powerivq
Copy link
Contributor

@powerivq powerivq commented Feb 19, 2021

Use the vendor specific integration js rather than f.js that contains all the vendors.

@powerivq powerivq force-pushed the enable-vendors branch 2 times, most recently from ce2092c to 00eb481 Compare March 11, 2021 20:44
Copy link
Contributor

@micajuine-ho micajuine-ho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval for wg-components owned files.

@powerivq powerivq force-pushed the enable-vendors branch 7 times, most recently from 3b06a6d to 6068e53 Compare March 14, 2021 05:21
@estherkim estherkim requested review from rsimha and removed request for estherkim March 17, 2021 20:51
@estherkim
Copy link
Collaborator

reassigning to @rsimha for build changes :)

Copy link
Contributor

@rsimha rsimha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build changes LGTM.

@powerivq powerivq merged commit f6a2f18 into ampproject:master Mar 22, 2021
@powerivq powerivq deleted the enable-vendors branch March 22, 2021 19:07
@rsimha
Copy link
Contributor

rsimha commented Mar 22, 2021

@powerivq Curious: Were the build changes in this PR reverted? (Looks like the original commit was amended after it was reviewed, which makes it hard to tell if / why something changed.)

@powerivq
Copy link
Contributor Author

@rsimha Yes they are reverted. There was merge conflict and have been reverted during rebasing so there is no history of that.

caroqliu added a commit to caroqliu/amphtml that referenced this pull request Apr 1, 2021
caroqliu added a commit to caroqliu/amphtml that referenced this pull request Apr 8, 2021
caroqliu added a commit that referenced this pull request Apr 13, 2021
* Prototype

* Remove loader comment and unused firstLayoutCompleted

* Move sandbox flags to src/core/3p-frame

* Move name to Preact layer

* .

* Replace getFrameAttributes

* ProxyIframeEmbed, IframeEmbed, Instagram refactor

* Bring back InstagramDef.Api

* Undo export

* Move src to ProxyIframeEmbed

* Fix imports

* Restore Instagram props

* Fix typing

* Take messageHandler instead

* Give messageHandler from Twitter

* Use forwardRef

* Clarify comment

* Dima comments

* Update from changes in #32763

* Add unit tests

* Update setHeightStyle

* Use useMemo for count

* getPreconnects returns array

* Add 3p-frame and iframe files to eslint allowlist

* height as string not object

* Extract helper

* Remove win from render body

* Calculate name and src together

* `src` should not be in the dependency array

* Remove options

* Update src/preact/component/iframe.js

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
rileyajones pushed a commit to rileyajones/amphtml that referenced this pull request Apr 16, 2021
* Prototype

* Remove loader comment and unused firstLayoutCompleted

* Move sandbox flags to src/core/3p-frame

* Move name to Preact layer

* .

* Replace getFrameAttributes

* ProxyIframeEmbed, IframeEmbed, Instagram refactor

* Bring back InstagramDef.Api

* Undo export

* Move src to ProxyIframeEmbed

* Fix imports

* Restore Instagram props

* Fix typing

* Take messageHandler instead

* Give messageHandler from Twitter

* Use forwardRef

* Clarify comment

* Dima comments

* Update from changes in ampproject#32763

* Add unit tests

* Update setHeightStyle

* Use useMemo for count

* getPreconnects returns array

* Add 3p-frame and iframe files to eslint allowlist

* height as string not object

* Extract helper

* Remove win from render body

* Calculate name and src together

* `src` should not be in the dependency array

* Remove options

* Update src/preact/component/iframe.js

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
rochapablo pushed a commit to rochapablo/amphtml that referenced this pull request Aug 30, 2021
* Prototype

* Remove loader comment and unused firstLayoutCompleted

* Move sandbox flags to src/core/3p-frame

* Move name to Preact layer

* .

* Replace getFrameAttributes

* ProxyIframeEmbed, IframeEmbed, Instagram refactor

* Bring back InstagramDef.Api

* Undo export

* Move src to ProxyIframeEmbed

* Fix imports

* Restore Instagram props

* Fix typing

* Take messageHandler instead

* Give messageHandler from Twitter

* Use forwardRef

* Clarify comment

* Dima comments

* Update from changes in ampproject#32763

* Add unit tests

* Update setHeightStyle

* Use useMemo for count

* getPreconnects returns array

* Add 3p-frame and iframe files to eslint allowlist

* height as string not object

* Extract helper

* Remove win from render body

* Calculate name and src together

* `src` should not be in the dependency array

* Remove options

* Update src/preact/component/iframe.js

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants