-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Restore embed-gist-inline feature
#3640
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
Conversation
|
One sec I am getting lost in the comments |
This reverts commit f5425cd.
Co-authored-by: Federico <me@fregante.com>
| // Get the gist via background.js due to CORB policies introduced in Chrome 73 | ||
| const gistData = await browser.runtime.sendMessage({ | ||
| action: 'fetch', | ||
| payload: `${link.href}.json` |
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.
Can we shorten this object to just being {fetch: 'blah blah json'}. Splitting payload from the action seems unnecessary (even though that’s how I’ve done it for years)
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.
I tried doing this and typescript did not like me. Want me to commit?
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.
Also the official google docs say to do a check first so that any sendmessage that is passed does not end up by mistake fetched
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.
What kind of check?
What does TypeScript say? It’s the same type, except that instead of calling the property payload it calls it fetch. That’s it.
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.
The problem was that fetch is a reserved word.
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.
Then got
Jokes aside, I think request is a better word
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.
The problem was that
fetchis a reserved word.
Ah the problem is in the destructuring part I suppose
|
Code looks good. Hope you tested the latest version in both browsers |
|
I did |
LINKED ISSUES:
Fixes
embed-gist-inlinefails #2022TEST URLS:
https://gist.github.com/sompylasar/99b5d307da3168b833c1119fb95caf11
None