Skip to content

Update for IMA v3.697.0#72898

Merged
typescript-bot merged 13 commits intoDefinitelyTyped:masterfrom
Kiro705:update
Jun 4, 2025
Merged

Update for IMA v3.697.0#72898
typescript-bot merged 13 commits intoDefinitelyTyped:masterfrom
Kiro705:update

Conversation

@Kiro705
Copy link
Copy Markdown
Contributor

@Kiro705 Kiro705 commented May 29, 2025

Updates google_interactive_media_ads_types for version IMA SDK v3.697.0.

Changes:

  • Fixes AdEvent.SKIPPED to the correct string 'skip'.
  • Improves definitions for AdsLoader methods 'addEventListener()' and 'removeEventListener()'.
  • Removes 'AdsLoader.dispatchEvent()', which is not needed for IMA implementations.
  • Improves definitions for AdsManager methods 'addEventListener()' and 'removeEventListener()'.
  • Updates comment for AdsManager methods 'expand()', 'init()', and 'resize()'.
  • Updates comment for AdsRenderingSettings values 'restoreCustomPlaybackStateOnAdBreakComplete' and 'uiElements'.
  • Updates comment for AdsRenderingSettings values 'linearAdSlotHeight', 'linearAdSlotWidth', 'nonLinearAdSlotHeight', and 'nonLinearAdSlotWidth'.
  • Updates comment for 'ImaEvent.target'.
  • Removes 'ImaEventTarget' interface.
  • Removes 'ViewMode' enum.
  • Deprecates ImaEvent 'currentTarget' and 'preventDefault()'.
  • Deprecates 'OmidAccessMode.DOMAIN'.
  • Adds blank lines between entries for better readability.
  • Adds 'google' prefix to comment strings like 'google.ima.AdEvent'.
  • Fixes comment styling for lists.
  • Updates google_interactive_media_ads_types version to match IMA SDK versions.

Please fill in this template.

Select one of these and delete the others:

If changing an existing definition:

  • Provide a URL to documentation or source code which provides context for the suggested changes: <>
  • If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the package.json.

@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented May 29, 2025

@Kiro705 Thank you for submitting this PR!

This is a live comment that I will keep updated.

1 package in this PR

Code Reviews

This PR can be merged once it's reviewed by a DT maintainer.

You can test the changes of this PR in the Playground.

Status

  • ✅ No merge conflicts
  • ✅ Continuous integration tests have passed
  • ✅ Only a DT maintainer can approve changes without tests

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.


Diagnostic Information: What the bot saw about this PR
{
  "type": "info",
  "now": "-",
  "pr_number": 72898,
  "author": "Kiro705",
  "headCommitOid": "6279ea6439924ca62a67f7bdb166805794e50bbd",
  "mergeBaseOid": "f2fcf51a0c379dc8daca803be36df971c14d5078",
  "lastPushDate": "2025-05-29T16:00:57.000Z",
  "lastActivityDate": "2025-06-04T13:46:52.000Z",
  "mergeOfferDate": "2025-06-03T17:13:20.000Z",
  "mergeRequestDate": "2025-06-04T13:46:52.000Z",
  "mergeRequestUser": "Kiro705",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Well-liked by everyone",
  "pkgInfo": [
    {
      "name": "google_interactive_media_ads_types",
      "kind": "edit",
      "files": [
        {
          "path": "types/google_interactive_media_ads_types/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/google_interactive_media_ads_types/package.json",
          "kind": "package-meta-ok"
        }
      ],
      "owners": [
        "Kiro705",
        "gschoppe"
      ],
      "addedOwners": [],
      "deletedOwners": [
        "gschoppe"
      ],
      "popularityLevel": "Well-liked by everyone"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "jakebailey",
      "date": "2025-06-03T17:12:41.000Z",
      "isMaintainer": true
    }
  ],
  "mainBotCommentID": 2919866250,
  "ciResult": "pass"
}

@typescript-bot typescript-bot added Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. New Definition This PR creates a new definition package. Check Config Changes a module config files labels May 29, 2025
@typescript-bot
Copy link
Copy Markdown
Contributor

🔔 @Kiro705 — you're the only owner, but it would still be good if you find someone to review this PR in the next few days, otherwise a maintainer will look at it. (And if you do find someone, maybe even recruit them to be a second owner to make future changes easier...)

@typescript-bot typescript-bot moved this to Needs Author Action in Pull Request Status Board May 29, 2025
@typescript-bot
Copy link
Copy Markdown
Contributor

@Kiro705 Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day!

@typescript-bot typescript-bot added Edits Owners This PR adds or removes owners Author is Owner The author of this PR is a listed owner of the package. Untested Change This PR does not touch tests and removed Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. New Definition This PR creates a new definition package. Check Config Changes a module config files labels May 29, 2025
@typescript-bot
Copy link
Copy Markdown
Contributor

Hey @Kiro705,

😒 Your PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Please consider adding tests to cover the change you're making. Including tests allows this PR to be merged by yourself and the owners of this module.

This can potentially save days of time for you!

@typescript-bot typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in Pull Request Status Board May 29, 2025
@typescript-bot
Copy link
Copy Markdown
Contributor

🔔 @gschoppe — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in Pull Request Status Board May 29, 2025
@typescript-bot typescript-bot moved this from Needs Maintainer Review to Waiting for Code Reviews in Pull Request Status Board May 29, 2025
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in Pull Request Status Board May 29, 2025
"name": "@types/google_interactive_media_ads_types",
"nonNpm": true,
"nonNpmDescription": "Google Interactive Media Ads HTML5 SDK",
"version": "1.20241219.9999",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This versioning goes backwards and so nobody will get this update. Did you mean to set a different version?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the version should change. In the initial release I mistakenly used the date to create the version number, but I think it makes more sense to match this version number to the IMA SDK version number (https://developers.google.com/interactive-media-ads/docs/sdks/html5/client-side/history)

Would it be reasonable to instead change the major number as well, so the new version would be "2.36970.9999"?

Open to other suggestions as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes. You'd have to bump the major.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I updated the version to 2.36970.9999. Moving forward, the version number will continue to match the latest IMA SDK version number.

Copy link
Copy Markdown
Member

@jakebailey jakebailey Jun 3, 2025

Choose a reason for hiding this comment

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

Seems fine, but why not just 3.697?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I ask mainly because you've mapped 3.697.0 to 36970, which if bumped according to semver on your side, could turn into 3.697.10, so 369710, which would be out of order again when 3.698.0 comes around again and is mapped to 36980. It seems like it would make more sense to just have the types version match your package's version?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I could do that as well. The IMA SDK version is 3.697.0, would it make more sense to use 3.6970.9999 for the type definition version?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

3.697.9999 would make the most sense, as it's unlikely a patch version would change an API. This is how regular npm packages are tracked (as they have semver versions too).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I'll update it to 3.697.9999. I agree that patch version can be ignored since it shouldn't change the APIs. Thanks for the help.

@typescript-bot typescript-bot added the Revision needed This PR needs code changes before it can be merged. label May 29, 2025
@typescript-bot typescript-bot moved this from Needs Maintainer Review to Needs Author Action in Pull Request Status Board May 29, 2025
@typescript-bot
Copy link
Copy Markdown
Contributor

@Kiro705 One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!

@typescript-bot typescript-bot removed the Revision needed This PR needs code changes before it can be merged. label Jun 3, 2025
@typescript-bot typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in Pull Request Status Board Jun 3, 2025
@typescript-bot
Copy link
Copy Markdown
Contributor

@jakebailey Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in Pull Request Status Board Jun 3, 2025
@Kiro705 Kiro705 requested a review from jakebailey June 3, 2025 16:45
@typescript-bot typescript-bot moved this from Needs Maintainer Review to Waiting for Code Reviews in Pull Request Status Board Jun 3, 2025
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in Pull Request Status Board Jun 3, 2025
@typescript-bot typescript-bot added Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner labels Jun 3, 2025
@typescript-bot
Copy link
Copy Markdown
Contributor

@Kiro705: Everything looks good here. I am ready to merge this PR (at 6279ea6) on your behalf whenever you think it's ready.

If you'd like that to happen, please post a comment saying:

Ready to merge

and I'll merge this PR almost instantly. Thanks for helping out! ❤️

(@gschoppe: you can do this too.)

@typescript-bot typescript-bot moved this from Needs Maintainer Review to Waiting for Author to Merge in Pull Request Status Board Jun 3, 2025
@Kiro705
Copy link
Copy Markdown
Contributor Author

Kiro705 commented Jun 4, 2025

Ready to merge

@typescript-bot typescript-bot moved this from Waiting for Author to Merge to Recently Merged in Pull Request Status Board Jun 4, 2025
@typescript-bot typescript-bot merged commit e91a5e9 into DefinitelyTyped:master Jun 4, 2025
4 checks passed
@Kiro705 Kiro705 deleted the update branch June 4, 2025 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Author is Owner The author of this PR is a listed owner of the package. Edits Owners This PR adds or removes owners Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner Untested Change This PR does not touch tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants