Skip to content
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

[core & settings-view] Avoid network requests for bundled packages #711

Merged
merged 6 commits into from
Sep 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"main": "./src/main-process/main.js",
"repository": {
"type": "git",
"url": "https://github.com/pulsar-edit/pulsar.git"
"url": "https://github.com/pulsar-edit/pulsar"
},
"bugs": {
"url": "https://github.com/pulsar-edit/pulsar/issues"
Expand Down
55 changes: 32 additions & 23 deletions packages/settings-view/lib/package-card.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import {CompositeDisposable, Disposable} from 'atom'
import {shell} from 'electron'
import etch from 'etch'
import BadgeView from './badge-view'
import path from 'path'

import {ownerFromRepository} from './utils'
import {ownerFromRepository, repoUrlFromRepository} from './utils'

let marked = null

Expand Down Expand Up @@ -280,31 +281,39 @@ export default class PackageCard {
}

loadCachedMetadata () {
this.client.avatar(ownerFromRepository(this.pack.repository), (err, avatarPath) => {
if (!err && avatarPath) {
this.refs.avatar.src = `file://${avatarPath}`
}
})

this.client.package(this.pack.name, (err, data) => {
// We don't need to actually handle the error here, we can just skip
// showing the download count if there's a problem.
if (!err) {
if (data == null) {
data = {}
if (repoUrlFromRepository(this.pack.repository) === atom.branding.urlCoreRepo) {
// Don't hit the web for our bundled packages. Just use the local image.
this.refs.avatar.src = `file://${path.join(process.resourcesPath, "pulsar.png")}`;
} else {
this.client.avatar(ownerFromRepository(this.pack.repository), (err, avatarPath) => {
if (!err && avatarPath) {
this.refs.avatar.src = `file://${avatarPath}`
}
})
}

if (this.pack.apmInstallSource && this.pack.apmInstallSource.type === 'git') {
this.refs.downloadIcon.classList.remove('icon-cloud-download')
this.refs.downloadIcon.classList.add('icon-git-branch')
this.refs.downloadCount.textContent = this.pack.apmInstallSource.sha.substr(0, 8)
} else {

this.refs.stargazerCount.textContent = data.stargazers_count ? parseInt(data.stargazers_count).toLocaleString() : ''
this.refs.downloadCount.textContent = data.downloads ? parseInt(data.downloads).toLocaleString() : ''
// We don't want to hit the API for this data, if it's a bundled package
if (this.pack.repository !== atom.branding.urlCoreRepo) {
this.client.package(this.pack.name, (err, data) => {
// We don't need to actually handle the error here, we can just skip
// showing the download count if there's a problem.
if (!err) {
if (data == null) {
data = {}
}

if (this.pack.apmInstallSource && this.pack.apmInstallSource.type === 'git') {
this.refs.downloadIcon.classList.remove('icon-cloud-download')
this.refs.downloadIcon.classList.add('icon-git-branch')
this.refs.downloadCount.textContent = this.pack.apmInstallSource.sha.substr(0, 8)
} else {

this.refs.stargazerCount.textContent = data.stargazers_count ? parseInt(data.stargazers_count).toLocaleString() : ''
this.refs.downloadCount.textContent = data.downloads ? parseInt(data.downloads).toLocaleString() : ''
}
}
}
})
})
}
}

updateInterfaceState () {
Expand Down
22 changes: 21 additions & 1 deletion packages/settings-view/lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,26 @@ const ownerFromRepository = repository => {
return match ? match[1] : ''
}

const repoUrlFromRepository = repository => {
if (!repository) return ''

let repo = repository

if (typeof repository === 'string') {
repo = repository
} else if (typeof repository === 'object' && typeof repository.url === 'string') {
repo = repository.url
} else {
repo = ''
}

if (repo.endsWith('.git')) {
repo = repo.replace('.git', '')
}

return repo
}

const packageComparatorAscending = (left, right) => {
const leftStatus = atom.packages.isPackageDisabled(left.name)
const rightStatus = atom.packages.isPackageDisabled(right.name)
Expand All @@ -37,4 +57,4 @@ const packageComparatorAscending = (left, right) => {
}
}

module.exports = {ownerFromRepository, packageComparatorAscending}
module.exports = {ownerFromRepository, repoUrlFromRepository, packageComparatorAscending}
3 changes: 2 additions & 1 deletion src/atom-environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,8 @@ class AtomEnvironment {
name: packagejson.branding.name,
urlWeb: packagejson.branding.urlWeb,
urlGH: packagejson.branding.urlGH,
urlForum: packagejson.branding.urlForum
urlForum: packagejson.branding.urlForum,
urlCoreRepo: packagejson.repository.url
confused-Techie marked this conversation as resolved.
Show resolved Hide resolved
};

// Keep instances of HistoryManager in sync
Expand Down
Loading