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

Fix rendering of chrome.gcm senderIds type. #52

Merged
merged 2 commits into from
Jun 7, 2023
Merged

Conversation

oliverdunk
Copy link
Member

Overview

Improves rendering of the chrome.gcm senderIds type in the chrome.gcm.register function.

Old New
Screenshot 2023-06-07 at 11 01 39 Screenshot 2023-06-07 at 11 00 44

Thought Process

When generating the chrome.gcm docs, we were previously seeing the following in the gcm.json spec:

{
  "name": "senderIds",
  "type": "array",
  "items": {
    "type": "string",
    "minLength": 1
  },
  "minItems": 1,
  "maxItems": 100,
},

And generating a union of all possible array lengths.

While it would be nice to have proper rendering for this in the future (e.g a min and max length tag) this PR implements a temporary solution where we simply collapse the type to X[] when we see more than 10 (an arbitrary but reasonable sound number) possible types. This seems reasonable since chrome.gcm is currently the only API with this problem (I checked) and the comments already explain the limit in detail.

Additional Details

Fixes GoogleChrome/developer.chrome.com#1850
Fixes GoogleChrome/developer.chrome.com#5176

@codecov
Copy link

codecov bot commented Jun 7, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.52 🎉

Comparison is base (98aee76) 35.61% compared to head (8b68314) 36.14%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #52      +/-   ##
==========================================
+ Coverage   35.61%   36.14%   +0.52%     
==========================================
  Files          21       21              
  Lines        4015     4023       +8     
  Branches      185      188       +3     
==========================================
+ Hits         1430     1454      +24     
+ Misses       2585     2569      -16     
Impacted Files Coverage Δ
tools/lib/render-context.js 60.36% <100.00%> (+2.52%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@oliverdunk oliverdunk merged commit 5885bf7 into main Jun 7, 2023
@oliverdunk oliverdunk deleted the gcm-union-fix branch June 7, 2023 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants