OpenWPM
OpenWPM copied to clipboard
Remove our custom interface types and replace them with the @types/firefox-webext-browser ones
Since the official types now have a lot more type definitions we can get rid of our own WebRequestOnBeforeSendHeadersEventDetails. This way we no longer need to maintain them manually. We should also check which other types we can just use from the library
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/e06e65d28fa1e3376fffc9ae08f85a74053518d3/types/firefox-webext-browser/index.d.ts#L4803-L4864
I am a first time contributor. can you elaborate more on this issue?
Hey @LordReigns Do you have any previous experience with TypeScript? If you don't I'd suggest you look at our other good-first-bug
As an overview, we currently define a lot of types for the WebExtension API ourselves, which is a lot of maintenance effort that we don't need to have. An example would be this: https://github.com/mozilla/OpenWPM/blob/512018f852a2b554b5b08a72936f835adf6b7110/openwpm/Extension/webext-instrumentation/src/types/browser-web-request-event-details.ts#L53-L99
If you compare the two types you'll notice that the official types have a lot more attributes. This task would ask, that you delete all of our type definitions that exist in a more complete form in @types/firefox-webext-browser
Thank you @vringar for the response. Since my experience with typescript is low I'll be looking for another good-first-bug. If you don't mind could you recommend one that would suit a first time contributor.
Thank You. 😊
Maybe have a look at #797
If #800 looks interesting to @LordReigns, he can also take over that.
What is your opinion? @vringar I am happy to guide him through the necessary changes, I will also put 3 option into potential fixes where we can use WHERE
clause
After #797 #800 may be a good next step. But #797 is a high-impact and easy to do change, so I'd prioritize that.
Thank you @vringar . I'll start working on the issue #797 I'm interested to work on issue #800 too, after finishing #797. @ankushduacodes . Your guidance would be very much appreciated.
Thank you all for your time 😊
@vringar. I like to to work on this issue. As you explained delete all of our type definitions that exist in a more complete form in @types/firefox-webext-browser. Should the deleted ones be replaced?
Here is how I understood the task,
interface _OnBeforeRequestDetails {
/**
* The ID of the request. Request IDs are unique within a browser session. As a result, they could be used to relate different events of the same request.
*/
requestId: string;
url: string;
/** Standard HTTP method. */
method: string;
For example: since this interface WebRequestOnBeforeRequestEventDetails
exists in @types/firefox-webext-browser as above, I should delete the existing interface in this repository.
Clarification
@vringar
The replacement for the interface WebRequestOnBeforeRequestEventDetails
is browser.webRequest.onBeforeRequest.
How should I technically add this type. Do you mean that all the files where the previous interface was implemented should be replaced with the above or else only add it to the types ( If so could u explain the procedure).
Thank you
Sorry for the delay. @vringar.
This is the approach I have taken,
example:
Consider the present interface WebRequestOnBeforeRequestEventDetails
I have replaced the above interface with the definition in firefox-webext-browser, as follows
import _OnBeforeRequestDetails = browser.webRequest._OnBeforeRequestDetails;
export { _OnBeforeRequestDetails as WebRequestOnBeforeRequestEventDetails}
Can you confirm this approach, so I can finalize it.
Thank you.
Hey, sorry for dropping the ball on this. This seems good :) Unfortunately I don't know too much about TypeScript so I'm unsure if it's the best way.
Hey, sorry for dropping the ball on this. This seems good :) Unfortunately I don't know too much about TypeScript so I'm unsure if it's the best way.
Oh, Ok, Is there a test for this ? so I can make sure this doesn't cause any errors.
Thank You.
As long as you can run ./scripts/build-extension.sh
without any errors everything should be fine.
Hey @LordReigns Hope you started off well in the new year :) Did you ever file a PR for this? From what we have discussed you seemed pretty close to having this done. Or do you need any help with anything.
Hey @LordReigns Hope you started off well in the new year :) Did you ever file a PR for this? From what we have discussed you seemed pretty close to having this done. Or do you need any help with anything.
I couldn't work on it as I am busy with exams. Sorry about that. You can assign somebody else. @vringar
Can you still push and file PR with what you've done so far? This is not urgent. Best of luck with your exams.