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

✨ [ads2.bid] amp-ad ads2bid implementation #34806

Closed
wants to merge 1,348 commits into from

Conversation

github-profitclicks
Copy link
Contributor

amp-ad ads2bid implementation

cc ampproject/wg-monetization // from docs

@amp-owners-bot amp-owners-bot bot requested a review from lannka June 10, 2021 02:38
@github-profitclicks
Copy link
Contributor Author

github-profitclicks commented Jul 6, 2021

@lannka Hello! How long can we wait for approval?

@lannka lannka enabled auto-merge (squash) July 21, 2021 18:07
alanorozco and others added 26 commits November 16, 2021 20:36
…pproject#36947)

Partial for ampproject#36899

Refactor `{attrPrefix}` and `{type: 'date'}` so that they resolve to a `{parseAttrs}` config instead.

This allows extension bundles to include these parsing mechanisms independently, rather than sharing it as part of `PreactBaseElement`.

All Bento extensions are 0.03 kb to 0.25 kb smaller as a result. The `bento.js` binary should see similar savings once it includes shared dependencies.
Reduces compressed size of `amp-story` bundles by ~0.6K
* compiler: create buildDom for amp-carousel 0.1

* good shape for scrollableCarousel

* fully implemented

* fix 1 real bug and the tests

* cleantown

* add buildDom tests

* make hasPrev/hasNext private

* small cleanup

* satisfy weird closure rule around default args and destructuring

* address first round of rcebulko comments

* typos, small nits
* remove createPortal
    * replace toChildArray with the fn from preact
    * copy/paste forwardRef implementation
…6978)

* Fix batchedReads being shared by different callers

In the old code, `batchedRead` performed a file read, and hashed the contents with an input `optionsHash`. But `batchedRead` is designed to allow multiple callers to share the same file, and they may provide different `optionsHash`es. When this happened, only the first caller's read+`optionsHash` would be used, and the 2nd+ caller would receive the first caller's hashes!

This simple fix is to move this out of `batchedRead` and let that just _read_ and hash the file contents. If you want to rehash with options, do that afterwards.

* Fix incorrect reference to file hash

* Fix files with duplicated contents compiling incorrectly
* Compressed share icons

* Removed google+ assets

* Simplify twitter share icon

* Not fail with gplus

* Not block space for gplus
…mpproject#36757)

* Added examples for inline and remote config loading of json files.

* finished tests for shopping config

* removed unnescessary function call

* removed chai import

* removed unnesc assignment

* fixed prettier lint warnings

* added dependencies for store service

* cleanup up code and tests to optmize code flow

* cleaned up config files and test files

* added more refactoring of shopping story

* Updated test code with new tag id

* fixed formatting of remote file

* added remote config fiel formatting fix

* added visual diff test

* updated visual diff test

* updated unit tests to use public facing apis, removed dead code

* updated unit tests to use public facing apis, removed dead code

* updated unit tests to use public facing apis, removed dead code

* added page id

* added load config impl (both solutions: one exposed export public method, and another private moethd that gets called, fallback to the exported loadconfigimpl if suggested by the review team)

* removed export ()may need to put back if want to use public facing apit

* modified laodconfig to handle 404 errors

* removed unused var

* added dependency for request service

* added a few recotoring shareconfig menthod

* refactored code to use new request service.

* removed unused allow list entry

* added buidlcallback resolve

* added implicit return

* added in code refactos

* updated shoo[[ing state to shopping data

* removed promise.resolve

* recatored shopping and shared config unit test code

* added in config refactor logic

* streamlined commens and code

* fixed comment for circleci

* optmized code and unit tests

* added shared config code

* refactored dead code

* added typedefs

* added in refactor of social share widget

* added shopping story test features

* removed unnneded export

* udpated product images array

* removed trailign whitespace

* removed unused vars

* removed unused config for now

* added back in dev asserts

* code compactification

* stripped dev assert

* fixed social share config
* Use stable identifier mangling for properties

* Reverse id mangling

Maybe we'll hit better compression with longer `const a=` or `let a=` back references.
* Compressed share icons

* Removed google+ assets

* Simplify twitter share icon

* Not fail with gplus

* Not block space for gplus

* Added utf8
* Cleanup forwardRef implementation

* Fix bug with forwarding a null ref

Functional components are only ever called during render/diff, not dismount, so the ref should never be null.

* Don't depend on `self` (browser global)
PiperOrigin-RevId: 409990959

Co-authored-by: Googler <[email protected]>
samouri and others added 6 commits January 18, 2022 13:34
* compiler: use .ts files

* touchups

* surprise
* Update OWNERS for tsconfig and compiler

* rcebulko feedback

* lint
* internal changes

PiperOrigin-RevId: 417462074

* Open source wasm AMP validator

PiperOrigin-RevId: 417488889

* Use plain string rather than Symbol in validator js

PiperOrigin-RevId: 419907682

* Fix a bug that compares string against number

PiperOrigin-RevId: 419931677

* Make htmlparser dedupe node attributes deterministically.

Non-deterministic deduping results in non-deterministic amp validator behavior. E.g. if there are two "width" attributes with different values, the layout validation may or may not produce a specific width-related error (even though validator will always detect attribute duplication itself, and fail).

PiperOrigin-RevId: 421140196

Co-authored-by: Boxiao Cao <[email protected]>
Co-authored-by: Googler <[email protected]>
@github-profitclicks
Copy link
Contributor Author

@calebcordry, could you give me an answer, please? What is the problem with CircleCI?

#!/bin/bash -eo pipefail
curl -sS https://raw.githubusercontent.com/ampproject/amphtml/main/.circleci/compute_merge_commit.sh | bash
bash: line 1: 404:: command not found

Exited with code exit status 127
CircleCI received exit code 127

@github-profitclicks
Copy link
Contributor Author

Maybe we should just make a new merge request? This has been going on for a very long time...

@calebcordry
Copy link
Member

Sorry missed this. Something like:

git checkout main
git pull upstream main
git checkout <your_branch>
git rebase main
git push origin <your_branch>

should sync everything to HEAD of our main branch. This fixes most CI errors.

jshamble and others added 15 commits January 19, 2022 15:09
* added some shopping tag text specs

* added shopping tag text specs

* fixed shopping tag

* fixed shopping tag

* added visual diff test

* fixed nit, one liner comment

* added dynamic border radius for styling with two tag texts

* added better name

* added correct calls for clientrect for using measuremutate eleemnt

* updated spec exampel to have tags with rtl support, used arabic as the demonstration language.

* fixed a few issues with shopping tag specs

* resolved mrege conflicts iwth main

* removed duplicate page id

* fixed blank page on no products

* added suggestion about ?. null operator

* added text specs

* added parseInt to font size to fix border radius

* fixed all issues except for multi-line ellipsis

* added shopping tag text specs ellipsis fix (it had to do with flex display)

* rmeoved gitignore file

* added mutate element

* added more relvant example

* updated visual diff tests

* added classlist optimization for mutatelement

* type

* fix circleCI tests
ESLint API changed so that `node.start/node.end` do not exist anymore. 

This makes the lint rule fail opaquely:

```
[13:02:52] AssertionError [ERR_ASSERTION]: Fix has invalid range: {
  "range": [
    null,
    null
  ],
  "text": ""
}
```

These values are now an array `node.range`. Correct errors after fix:

```
build-system/test-configs/forbidden-terms.js
  737:7  error  File does not exist  local/forbidden-terms-config
```
…ct#37420)

* set and remove attribute

* remove toggle

* Use toggleAttribute utility.

* Add toggleAttribute to forbidden terms

* Refactor toggle attribute.
* extract fn to register custom element

* create (empty) amp pbe

* inherit amp-accordion from amp-pbe

* fix custom-element definitions for iframes (fixes tests)

* workarounds to properly test bento components

we don't want AMP.BaseElement to be declared so that it won't get injected into the class hierarchy. current tests assume that AMP should be initialized all the time which breaks bento tests

* split out generic bento testing from amp tests

current (hacky) workarounds: some tests depend on CSS, which we expect consumers to install. This differs from the AMP components that register styles themselves. To workaround this we have to import and append the bento component styles. This problem might be unique to components that don't use shadowCss. For components that do use shadowDom and have critical styling that needs to be tested, those styles are already written in JSS and declared as shadowCss in the bento component's declaration, which ensures that that the styles are baked into the component.

* Revert "workarounds to properly test bento components"

un-revert in order to test bento component

* inherit all non-video bento-amp components from amp-pbe

* rename VideoBaseElement -> AmpVideoBaseElement

* refactor inheritance hierarchy for video components to remove amp code from bento hierarchy

* fix amp-facebook hierarchy to preserve AmpFbBase functionality

* update docs to demonstrate new way to scaffold amp-* components

* move (some) amp-specific methods into amp-pbe

* update dependencies, rename BentoVideoBaseElement

* add doc

* refactor dailymotion

* ampvideobaseelement docs

* lint

* fix typings and lint

* remove file

* remove dict() based on ongoign discussions to deprecate

* disable local/restrict-this-access for the whole file

* remove more references to dict()
auto-merge was automatically disabled January 21, 2022 14:22

Head branch was pushed to by a user without write access

@amp-owners-bot
Copy link

amp-owners-bot bot commented Jan 21, 2022

Hey @rsimha! These files were changed:

.circleci/check_config.sh
.circleci/compute_merge_commit.sh
.circleci/config.yml
.circleci/fail_fast.sh
.circleci/fetch_merge_commit.sh
.circleci/get_pinned_chrome_version.sh
.circleci/get_pr_number.sh
.circleci/initialize_repo.sh
.circleci/install_dependencies.sh
.circleci/install_microsoft_edge.sh
.circleci/install_validator_dependencies.sh
.circleci/maybe_gracefully_halt.sh
+56 more

Hey @erwinmombay, @jridgewell! These files were changed:

build-system/babel-plugins/babel-plugin-amp-mode-transformer/index.js
build-system/babel-plugins/babel-plugin-amp-mode-transformer/test/fixtures/transform-assertions/no-transform/input.js
build-system/babel-plugins/babel-plugin-amp-mode-transformer/test/fixtures/transform-assertions/no-transform/options.json
build-system/babel-plugins/babel-plugin-amp-mode-transformer/test/fixtures/transform-assertions/no-transform/output.js
build-system/babel-plugins/babel-plugin-amp-mode-transformer/test/fixtures/transform-assertions/nomodule/input.js
build-system/babel-plugins/babel-plugin-amp-mode-transformer/test/fixtures/transform-assertions/nomodule/options.json
build-system/babel-plugins/babel-plugin-amp-mode-transformer/test/fixtures/transform-assertions/nomodule/output.mjs
build-system/babel-plugins/babel-plugin-amp-mode-transformer/test/fixtures/transform-assertions/transform/input.js
build-system/babel-plugins/babel-plugin-amp-mode-transformer/test/fixtures/transform-assertions/transform/options.json
build-system/babel-plugins/babel-plugin-amp-mode-transformer/test/fixtures/transform-assertions/transform/output.mjs
build-system/babel-plugins/babel-plugin-amp-mode-transformer/test/index.js
build-system/babel-plugins/babel-plugin-const-transformer/index.js
+403 more

Hey @alanorozco! These files were changed:

build-system/compile/generate/OWNERS
build-system/compile/generate/bento.js
build-system/compile/generate/shared-bento-symbols.js
build-system/compile/generate/test/bento.test.js
build-system/compile/generate/test/shared-bento-symbols.test.js
build-system/server/app-index/amphtml-helpers.js
build-system/server/app-index/basepath-mappings.js
build-system/server/app-index/boilerplate.js
build-system/server/app-index/document-modes.js
build-system/server/app-index/file-list.js
build-system/server/app-index/form.js
build-system/server/app-index/header-links.js
+38 more

Hey @danielrozenberg! These files were changed:

build-system/compile/internal-version.js
build-system/tasks/visual-diff/browser.js
build-system/tasks/visual-diff/consts.js
build-system/tasks/visual-diff/dev-mode.js
build-system/tasks/visual-diff/helpers.js
build-system/tasks/visual-diff/index.js
build-system/tasks/visual-diff/log.js
build-system/tasks/visual-diff/package-lock.json
build-system/tasks/visual-diff/package.json
build-system/tasks/visual-diff/snippets/freeze-canvas-image.js
build-system/tasks/visual-diff/snippets/freeze-form-values.js
build-system/tasks/visual-diff/snippets/iframe-wrapper.js
+4 more

Hey @rcebulko! These files were changed:

build-system/tasks/check-types.js

Hey @estherkim! These files were changed:

build-system/tasks/e2e/amp-driver.js
build-system/tasks/e2e/controller-promise.js
build-system/tasks/e2e/describes-e2e.js
build-system/tasks/e2e/driver/query-xpath.js
build-system/tasks/e2e/e2e-types.js
build-system/tasks/e2e/expect.js
build-system/tasks/e2e/helper.js
build-system/tasks/e2e/index.js
build-system/tasks/e2e/mocha-ci-reporter.js
build-system/tasks/e2e/mocha-custom-json-reporter.js
build-system/tasks/e2e/mocha-dots-reporter.js
build-system/tasks/e2e/network-logger.js
+4 more

Hey @rileyajones! These files were changed:

build-system/tsconfig.json

Hey @ampproject/wg-caching! These files were changed:

extensions/amp-3d-gltf/0.1/test/validator-amp-3d-gltf.html
extensions/amp-3d-gltf/0.1/test/validator-amp-3d-gltf.out
extensions/amp-3d-gltf/validator-amp-3d-gltf.protoascii
extensions/amp-3q-player/0.1/test/validator-amp-3q-player.html
extensions/amp-3q-player/0.1/test/validator-amp-3q-player.out
extensions/amp-3q-player/validator-amp-3q-player.protoascii
extensions/amp-access-fewcents/0.1/test/validator-amp-access-fewcents.html
extensions/amp-access-fewcents/0.1/test/validator-amp-access-fewcents.out
extensions/amp-access-laterpay/0.1/test/validator-amp-access-laterpay.html
extensions/amp-access-laterpay/0.1/test/validator-amp-access-laterpay.out
extensions/amp-access-laterpay/0.2/test/validator-amp-access-laterpay.html
extensions/amp-access-laterpay/0.2/test/validator-amp-access-laterpay.out
+197 more

Hey @ampproject/wg-amp4email! These files were changed:

extensions/amp-accordion/validator-amp-accordion.protoascii
extensions/amp-anim/validator-amp-anim.protoascii
extensions/amp-autocomplete/validator-amp-autocomplete.protoascii
extensions/amp-bind/validator-amp-bind.protoascii
extensions/amp-carousel/validator-amp-carousel.protoascii
extensions/amp-fit-text/validator-amp-fit-text.protoascii
extensions/amp-form/validator-amp-form.protoascii

Hey @Jiaming-X! These files were changed:

extensions/amp-ad-network-adsense-impl/0.1/adsense-shared-state.js
extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js
extensions/amp-ad-network-adsense-impl/0.1/responsive-state.js
extensions/amp-ad-network-adsense-impl/0.1/test/test-amp-ad-network-adsense-impl.js
extensions/amp-ad-network-adsense-impl/0.1/test/test-responsive-state.js
extensions/amp-ad-network-adsense-impl/amp-ad-network-adsense-impl-internal.md

Hey @jeffkaufman! These files were changed:

extensions/amp-ad-network-adsense-impl/0.1/adsense-shared-state.js
extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js
extensions/amp-ad-network-adsense-impl/0.1/responsive-state.js
extensions/amp-ad-network-adsense-impl/0.1/test/test-amp-ad-network-adsense-impl.js
extensions/amp-ad-network-adsense-impl/0.1/test/test-responsive-state.js
extensions/amp-ad-network-adsense-impl/amp-ad-network-adsense-impl-internal.md
extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js
extensions/amp-ad-network-doubleclick-impl/0.1/flexible-ad-slot-utils.js
extensions/amp-ad-network-doubleclick-impl/0.1/safeframe-host.js
extensions/amp-ad-network-doubleclick-impl/0.1/sra-utils.js
extensions/amp-ad-network-doubleclick-impl/0.1/test/test-amp-ad-network-doubleclick-impl.js
extensions/amp-ad-network-doubleclick-impl/0.1/test/test-doubleclick-fluid.js
+15 more

Hey @dianomi-cjas! These files were changed:

extensions/amp-ad-network-dianomi-impl/0.1/amp-ad-network-dianomi-impl.js
extensions/amp-ad-network-dianomi-impl/0.1/test/test-dianomi-a4a-config.js
extensions/amp-ad-network-dianomi-impl/OWNERS
extensions/amp-ad-network-dianomi-impl/amp-ad-network-dianomi-impl-internal.md

Hey @smart-adserver! These files were changed:

extensions/amp-ad-network-smartadserver-impl/0.1/amp-ad-network-smartadserver-impl.js
extensions/amp-ad-network-smartadserver-impl/0.1/test/test-amp-ad-network-smartadserver-impl.js
extensions/amp-ad-network-smartadserver-impl/OWNERS
extensions/amp-ad-network-smartadserver-impl/amp-ad-network-smartadserver-impl-internal.md
extensions/amp-ad-network-smartadserver-impl/readme.md

Hey @zvalmog! These files were changed:

extensions/amp-analytics/0.1/vendors/appsflyer.json

Hey @gmajoulet, @mszylkowski! These files were changed:

extensions/amp-cache-url/0.1/amp-cache-url.js
extensions/amp-cache-url/0.1/test/test-amp-cache-url.js
extensions/amp-cache-url/0.1/test/validator-amp-cache-url.html
extensions/amp-cache-url/0.1/test/validator-amp-cache-url.out
extensions/amp-cache-url/validator-amp-cache-url.protoascii

@github-profitclicks
Copy link
Contributor Author

@calebcordry thank you! But now we are facing problems. Could you explain the reason?

1. End-to-end test

amp-bind chrome Standalone environment "before each" hook for "should support binding to src"

"before each" hook for "should support binding to src"
/home/circleci/project/test/e2e/test-amp-bind-iframe.js
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/circleci/project/test/e2e/test-amp-bind-iframe.js)
    at listOnTimeout (node:internal/timers:557:17)
    at processTimers (node:internal/timers:500:7)
  1. codecov/patch codec failure/correction after 1 second — 72.97% of differences (target 75.00%)

  2. Owners-Check Recommended Reviewers: Ridgewell

@calebcordry
Copy link
Member

Yeah it looks like that didnt quite work as expected. It might be easier to start a new one now, or I can try to walk you through cleaning up the commits?

@github-profitclicks
Copy link
Contributor Author

@calebcordry thanks. We created new MR. Link: #37501

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.