added altair-crx #2757
Merged
added altair-crx #2757
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's Guide by SourceryThis pull request introduces a new package, Class diagram for RenderOptions changesclassDiagram
class RenderOptions {
+boolean|string serveInitialOptionsInSeperateRequest
+string baseURL
}
class AltairStatic {
+renderAltair(options: RenderOptions)
+renderInitialOptions(options: RenderOptions)
+renderInitSnippet(options: RenderOptions)
}
RenderOptions -- AltairStatic
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @imolorhe - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider removing commented out code blocks in vite.config.ts and background.ts to keep the codebase cleaner
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
20
to
30
| function createNewTab() { | ||
| chrome.tabs.create({ url: 'index.html' }, function(tab) { | ||
| curTab = { | ||
| id: tab.id, | ||
| url: tab.url, | ||
| }; | ||
|
|
||
| // Handle donation logic | ||
| // handleDonation(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
issue (code-quality): Avoid function declarations, favouring function assignment expressions, inside blocks. (avoid-function-declarations-in-blocks)
Explanation
Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers. Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you should use function expressions, which create functions in-scope.
Comment on lines
33
to
36
| function focusTab(tabId) { | ||
| const updateProperties = { active: true }; | ||
| chrome.tabs.update(tabId, updateProperties, function(tab) {}); | ||
| } |
There was a problem hiding this comment.
issue (code-quality): Avoid function declarations, favouring function assignment expressions, inside blocks. (avoid-function-declarations-in-blocks)
Explanation
Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers. Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you should use function expressions, which create functions in-scope.
Comment on lines
38
to
58
| function openChangeLog() { | ||
| chrome.storage.sync.get( | ||
| { | ||
| showChangeLog: true, | ||
| }, | ||
| function(items) { | ||
| if (items.showChangeLog) { | ||
| chrome.tabs.create( | ||
| { | ||
| url: 'https://altairgraphql.dev/updated', | ||
| }, | ||
| function(tab) { | ||
| console.log( | ||
| 'New tab launched with https://altairgraphql.dev/updated' | ||
| ); | ||
| } | ||
| ); | ||
| } | ||
| } | ||
| ); | ||
| } |
There was a problem hiding this comment.
issue (code-quality): Avoid function declarations, favouring function assignment expressions, inside blocks. (avoid-function-declarations-in-blocks)
Explanation
Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers. Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you should use function expressions, which create functions in-scope.
Comment on lines
60
to
88
| function handleDonation() { | ||
| if (!chrome.runtime.getBrowserInfo) { | ||
| // FIXME: A chrome extension | ||
| chrome.storage.sync.get( | ||
| { | ||
| userDonated: false, | ||
| extLoadCount: 0, | ||
| }, | ||
| function(items) { | ||
| if (!items.userDonated) { | ||
| console.log('extension loaded count: ', items.extLoadCount); | ||
| if (items.extLoadCount > MAX_EXT_LOAD_COUNT) { | ||
| // show donation page | ||
| chrome.tabs.create({ url: 'donate.html' }, function(tab) { | ||
| console.log('New tab launched with donation.'); | ||
| }); | ||
| chrome.storage.sync.set({ | ||
| extLoadCount: 0, | ||
| }); | ||
| } else { | ||
| chrome.storage.sync.set({ | ||
| extLoadCount: items.extLoadCount + 1, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
issue (code-quality): Avoid function declarations, favouring function assignment expressions, inside blocks. (avoid-function-declarations-in-blocks)
Explanation
Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers. Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you should use function expressions, which create functions in-scope.
packages/altair-crx/public/js/buy.js
Outdated
| @@ -0,0 +1,2 @@ | |||
| (function() { var f=this,g=function(a,d){var c=a.split("."),b=window||f;c[0]in b||!b.execScript||b.execScript("var "+c[0]);for(var e;c.length&&(e=c.shift());)c.length||void 0===d?b=b[e]?b[e]:b[e]={}:b[e]=d};var h=function(a){var d=chrome.runtime.connect("nmmhkkegccagdldgiimedpiccmgmieda",{}),c=!1;d.onMessage.addListener(function(b){c=!0;"response"in b&&!("errorType"in b.response)?a.success&&a.success(b):a.failure&&a.failure(b)});d.onDisconnect.addListener(function(){!c&&a.failure&&a.failure({request:{},response:{errorType:"INTERNAL_SERVER_ERROR"}})});d.postMessage(a)};g("google.payments.inapp.buy",function(a){a.method="buy";h(a)}); | |||
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
|
|
||
| function onPurchase(purchase) { | ||
| console.log('onPurchase', purchase); | ||
| const orderId = purchase.response.orderId; |
There was a problem hiding this comment.
suggestion (code-quality): Prefer object destructuring when accessing and using properties. (use-object-destructuring)
Suggested change
| const orderId = purchase.response.orderId; | |
| const {orderId} = purchase.response; |
Explanation
Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.From the Airbnb Javascript Style Guide
Comment on lines
23
to
27
| function onPurchaseFailed(purchase) { | ||
| console.log('onPurchaseFailed', purchase); | ||
| var reason = purchase.response.errorType; | ||
| resultSection.innerHTML = 'Donation failed. ' + reason; | ||
| } |
There was a problem hiding this comment.
issue (code-quality): Avoid function declarations, favouring function assignment expressions, inside blocks. (avoid-function-declarations-in-blocks)
Explanation
Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers. Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you should use function expressions, which create functions in-scope.|
|
||
| function onPurchaseFailed(purchase) { | ||
| console.log('onPurchaseFailed', purchase); | ||
| var reason = purchase.response.errorType; |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
| @@ -0,0 +1,28 @@ | |||
| // Saves options to chrome.storage | |||
| function save_options() { | |||
| var showChangeLog = document.getElementById('show_changelog').checked; | |||
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
| showChangeLog: showChangeLog | ||
| }, function () { | ||
| // Update status to let user know options were saved. | ||
| var status = document.getElementById('status'); |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
|
Visit the preview URL for this PR (updated for commit 9fa9898): https://altair-gql--pr2757-imolorhe-browser-ext-x42bdg23.web.app (expires Wed, 05 Feb 2025 07:19:02 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 02d6323d75a99e532a38922862e269d63351a6cf |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes
Fixes #2504
Checks
yarn test-buildChanges proposed in this pull request:
Summary by Sourcery
Add Altair Chrome extension.
New Features:
Tests: