Skip to content
This repository has been archived by the owner on Oct 13, 2023. It is now read-only.

Fix bug: table update error while using file view #53

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

msvamp
Copy link

@msvamp msvamp commented Jun 24, 2020

The extension tried to update file sizes even when a directory page was not being browsed, which led to error TypeError: tree.forEach is not a function on line 153 (line number before this pull request)
For example, visit page: https://github.com/harshjv/github-repo-size/blob/master/src/inject.js and check extension errors.

This is now fixed by detecting directory view with the presence of the 'Add file' button and updating table only if the button is present.

Also, removed redundant CSS 'cursor: pointer' for newLiElem.

Merging with latest changes
The extension tried to update file sizes even when a directory page was not being browsed, which led to error `TypeError: tree.forEach is not a function` on line 153 (line number before this commit)
For example, visit page: https://github.com/harshjv/github-repo-size/blob/master/src/inject.js and check extension errors

This is now fixed by detecting directory view with the presence of the 'Add file' button and updating table only if the button is present.

Also, removed redundant CSS 'cursor: pointer' for newLiElem.
This reverts commit c998132.

Undoing bugfix because file sizes on repo main page are broken
The extension tried to update file sizes even when a directory page was not being browsed, which led to error `TypeError: tree.forEach is not a function` on line 153 (line number before this commit)
For example, visit page: https://github.com/harshjv/github-repo-size/blob/master/src/inject.js and check extension errors

This is now fixed by detecting directory view with the presence of the 'Add file' button and updating table only if the button is present.

Also, removed redundant CSS 'cursor: pointer' for newLiElem.
src/inject.js Outdated Show resolved Hide resolved
Now using distinct classes for directory view detection as requested here: harshjv#53 (comment)
We will now detect the presence of rows in the files list box instead of using the 'Add file' button

Removed copied attributes from 'Settings' nav-item because it caused GitHub's native JS to throw errors in repos where the 'Settings' button is not present.

Updated CSS classes and width for SVG icon
@msvamp msvamp requested a review from harshjv June 26, 2020 15:39
@harshjv
Copy link
Owner

harshjv commented Sep 22, 2020

@msvamp thank you for the update!

Can you please share steps to reproduce TypeError: tree.forEach is not a function error?

@msvamp
Copy link
Author

msvamp commented Sep 23, 2020

Sure. The steps are pretty simple.

Steps to reproduce

  1. Ensure the extension is enabled and Chrome Extensions Developer mode is enabled
  2. If the unpacked extension is being run, skip this step. But if one is running the extension from the webstore, Chrome will ignore all errors by default. So, visit chrome://extensions/?id=apnjnioapinblneaedefcnopcjepgkci and enable the Collect Errors switch.
  3. Now, visit the page corresponding to any file (not folder) in any GitHub repository. For example, the license file of this repository.
  4. Go to chrome://extensions/?errors=apnjnioapinblneaedefcnopcjepgkci and the error should be there.

Error in Extensions section

Steps to reproduce (alternate)

Alternately, the same error is also visible in the console section of the Developer Tools on visiting the same page.
Error in Extensions section

What causes the error?

When a request is made to GitHub API for a directory in a repo or the root of the repo, the response contains the corresponding details of all files in the folder in the the form of an array of JSON objects.

Example request: https://api.github.com/repos/harshjv/github-repo-size/contents/?ref=f82f23b9a2c18afdd824cc0b89b1d909b9a090f9
API Response (folder)

But when a request is made for a file in a repo, the response contains a single JSON object with the details of the single file, and hence the forEach operation fails.

Example request: https://api.github.com/repos/harshjv/github-repo-size/contents/LICENSE.md?ref=f82f23b9a2c18afdd824cc0b89b1d909b9a090f9
API Response (file)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants