CustomEvent: pass a ReadTheDocsData object as event.detail#287
Conversation
src/readthedocs-config.js
Outdated
| EVENT_READTHEDOCS_ADDONS_DATA_READY, | ||
| document, | ||
| dataUser, | ||
| new ReadTheDocsEventData(dataUser), |
There was a problem hiding this comment.
And in terms of resolving the promise before calling the event, dispatchEvent would trigger inside a promise then() that resolves dataUser
There was a problem hiding this comment.
I didn't understand your comment here. Can you expand on this? 🙏🏼
There was a problem hiding this comment.
I read this comment a couple of times and I also tested this locally to find our how to resolve this Promise. I made these changes in a92994a. Let me know if that sounds good to you.
There was a problem hiding this comment.
So what I mean is that response.json()/dataUser is a promise, and we should give the user the actual value that is provided by resolving/fulfilling response.json(). You seem to have it close I think, but I feel like your example might be prematurely resolving the parent Promise perhaps. Let me digest this one a bit more.
There was a problem hiding this comment.
I just commented on the commit directly a92994a
But to keep conversation here, you may have resolved the wrong promises (both the API promises and the user JSON data promises).
I also think the class is not adding too much right. However, do you think we will have the same flexibility with a function in the future in case we need to perform more validations/checks? I'm asking because my limited js knowledge 😅 |
Call `.data()` only to get the data.
We would have plenty of flexibility to include logic that we want to hide from the maintainer user. But, if we wanted to expose and public functions/methods, the class structure would make this cleaner. I think it's fine to stick with the class based approach if we aren't sure or are leaning towards exposing public methods -- ie, maybe a |
|
It seems we both agree on passing a class instead of the raw JSON data. So, I'm merging this PR into the one its based, and we can continue with the minor details there. |
I found a pattern that I like 👍🏼 . I think it's a clear way to communicate users what's going on, but also leaves the door open to expand checks/validations in the future. I also imagine we could even want to know "What projects are subscribing to the custom event?" in case we need to contact them due to a breaking change or similar.
Examples
Calling
event.detail.initialize()without having defined the<meta ... />HTML tag:Calling
event.detail.data()without calling.initialize()first:Closes #279