-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: Add config for add headers to license requests #6650
Conversation
I need review the error: |
Incremental code coverage: 91.30% |
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.
Looks good to me, apart from those leftover console.log statements and any errors you're seeing in the demo
@joeyparrish can you help me to resolve it? |
I found the source of the problem. It's really a design problem. The configuration system wants to map out and type-check every input. However, some parts of the config are dictionaries with arbitrary keys. Ex: drm.advanced[keySystem]... We still want to type-check the values under those arbitrary keys, so the config system has this "overrides" system. It basically allows the config algorithm to skip a level in the hierarchy. A simplified example: const overrides = {
'.drm.servers': '',
'.drm.advanced': {
videoRobustness: '',
audioRobustness: '',
sessionType: '',
},
}; This means the path And the path The trouble is that const overrides = {
'.drm.advanced': {
videoRobustness: '',
audioRobustness: '',
sessionType: '',
headers: {},
},
}; Now I'm pondering the "best" solution, where "best" either means "cleanest", "most extensible", or "quickest"... |
I found a relatively quick solution... the merge system will now recognize "{}" in the template as "just an object, do a shallow copy and don't recurse" |
@avelad, please check the demo and confirm, then merge when you're ready. |
Yes it works, but now the tests are failing |
let genericObject = false; | ||
if (overrideTemplate) { | ||
genericObject = template.constructor == Object && | ||
Object.keys(overrides).length == 0; |
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.
This is not right. overrides
always has keys (eg ".drm.advanced"), and I don't see why you would check it here.
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.
It’s the only solution that I found to fix the tests
No description provided.