Skip to content

Commit e1fcfe1

Browse files
stefanbuckstefanbuck
authored
Prefetch urls in background and insert "real" anchor links (#543)
* Remove lazy loader for relative files * Add setting to enable support for private repositories * Insert real href tag * Add token field to settings page * Add octolinker api helper * Align format with new octolinker api * Use ZEIT now deployment * Split head from base sha in url In some cases the file path in PR comment contains head and a base sha like https://github.com/user/repo/pull/123/files/f18370bde83d7f1ced7acf5af54a21d2e53912ff..637d21dd86c8f50e9100b8da656e43cadcccde4d#diff-fd9ad2ea8184b3a5e7d76f54e87ae438 * Resolve urls with either the github or octolinker API * Update E2E test to work with href attribute * Correct grammar See #543 (comment) * Implement simple GitHub search * Remove option to toggle line indicator * Open settings dialog rather than being a new tab * Fix GitHub API call in Firefox * Simplify loader * Add trusted url resolver to reduce load on octolinker api * Fix tests
1 parent 3f07179 commit e1fcfe1

File tree

41 files changed

+486
-417
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+486
-417
lines changed

‎assets/manifest.json‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
},
2323
"options_ui": {
2424
"page": "options.html",
25-
"open_in_tab": true,
2625
"chrome_style": true
2726
},
2827
"content_scripts": [
@@ -42,6 +41,6 @@
4241
"storage",
4342
"https://github.com/",
4443
"https://gist.github.com/",
45-
"https://githublinker.herokuapp.com/"
44+
"https://octolinker.now.sh/"
4645
]
4746
}

‎e2e/automated.test.js‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,13 @@ const fixtures = require('./fixtures.json'); // eslint-disable-line import/no-un
44
const diffFixtures = require('./diff-fixtures.json'); // eslint-disable-line import/no-unresolved
55

66
async function executeTest(url, targetUrl, selector) {
7-
await page.goto(url);
7+
if ((await page.url()) !== url) {
8+
await page.goto(url);
9+
}
810

9-
await page.waitForSelector(selector);
10-
11-
await Promise.all([
12-
page.waitForNavigation(),
13-
// page.click(selector), for some reason page.click is not working
14-
page.$eval(selector, el => el.click()),
15-
]);
16-
17-
await expect(page.url()).toEqual(expect.stringMatching(targetUrl));
11+
await page.waitForSelector(`${selector}[href$="${targetUrl}"]`);
1812
}
1913

20-
jest.setTimeout(20000);
21-
2214
describe('End to End tests', () => {
2315
beforeAll(async () => {
2416
if (!process.env.E2E_USER_NAME || !process.env.E2E_USER_PASSWORD) {

‎package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@
1414
"test:watch": "jest --watch",
1515
"version": "json -I -f assets/manifest.json -e \"this.version='`json -f package.json version`'\" && git add assets/manifest.json && json -I -f packages/core/package.json -e \"this.version='`json -f package.json version`'\" && git add packages/core/package.json",
1616
"package": "web-ext build --source-dir dist --artifacts-dir out --overwrite-dest",
17+
"patch-permissions": "yarn json -I -f assets/manifest.json -e 'this.permissions.push(\"https://api.github.com/\")'",
1718
"release:cws": "cd dist && webstore upload --auto-publish",
18-
"release:amo": "cd dist && webext submit",
19+
"release:amo": "cd dist && yarn patch-permissions && webext submit",
1920
"release": "npm run release:cws && npm run release:amo",
2021
"build": "webpack --mode=production",
2122
"watch": "webpack --watch --mode=development",

‎packages/blob-reader/helper.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ function mergeRepoAndFilePath(repoPath, filePath) {
1818
const repoUrl = repoPath
1919
.trim()
2020
.split('#')[0]
21-
.replace(/pull\/[0-9]+\/files/, 'blob');
21+
.replace(/pull\/[0-9]+\/files/, 'blob')
22+
.split('..')[0];
2223

2324
return `${repoUrl}/${filePath.trim()}`;
2425
}

‎packages/core/__tests__/loader.test.js‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import loader from '../loader';
2+
3+
describe('loader', () => {
4+
const getMatches = (type, urls) => [
5+
{
6+
link: { href: '' },
7+
urls: [
8+
{
9+
type,
10+
urls,
11+
user: 'octolinker',
12+
repo: 'testrepo',
13+
branch: 'master',
14+
path: 'lib/index.js',
15+
},
16+
],
17+
},
18+
];
19+
20+
it('call github api to resolve internal links', () => {
21+
loader(
22+
getMatches('internal-link', [
23+
'https://github.com/octolinker/testrepo/blob/master/lib/index.js',
24+
]),
25+
);
26+
});
27+
it('call octolinker api to resolve external links', () => {});
28+
it('call github and octolinker api to resolve external and internal links', () => {});
29+
});

‎packages/core/background/index.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,9 @@ import newTab from './newTab';
44
storage.load().then(() => {
55
newTab();
66
});
7+
8+
chrome.runtime.onMessage.addListener(({ action }) => {
9+
if (action !== 'openSettings') return;
10+
11+
chrome.runtime.openOptionsPage();
12+
});

‎packages/core/click-handler.js‎

Lines changed: 0 additions & 133 deletions
This file was deleted.

‎packages/core/loader.js‎

Lines changed: 112 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,126 @@
1-
function createStore(json, payload) {
2-
const store = global.__ocotlinker_cache || {};
1+
import { fetchTree } from '@octolinker/helper-github-api';
2+
import { bulkAction } from '@octolinker/helper-octolinker-api';
33

4-
payload.forEach(({ registry, target }, index) => {
5-
store[registry] = store[registry] || {};
6-
store[registry][target] = json[index];
7-
});
8-
9-
global.__ocotlinker_cache = store;
4+
function getRepoMetadata(data) {
5+
// Find first internal links which contains the information we need
6+
// TODO handle also githubSearch resolver results. Right now this works,
7+
// because a relative file resolver is used as well in the less and sass plugin
8+
return data.find(({ type }) => type === 'internal-link') || {};
109
}
1110

12-
async function runLiveQuery(matches) {
13-
if (!matches.length) {
14-
return [];
15-
}
11+
function groupMatchesByType(matches) {
12+
const flattenUrls = [].concat(
13+
...matches.map(match =>
14+
match.urls.map(url => ({
15+
...url,
16+
link: match.link,
17+
})),
18+
),
19+
);
1620

17-
const payload = [].concat(...matches.map(match => match.urls));
21+
const apiItems = flattenUrls.filter(({ type }) =>
22+
['registry', 'ping'].includes(type),
23+
);
1824

19-
const response = await fetch('https://githublinker.herokuapp.com/bulk', {
20-
method: 'POST',
21-
body: JSON.stringify(payload),
22-
headers: new Headers({
23-
'Content-Type': 'application/json',
24-
}),
25-
});
26-
const json = await response.json();
25+
const internalItems = flattenUrls.filter(({ type }) =>
26+
['internal-link', 'github-search'].includes(type),
27+
);
2728

28-
createStore(json, payload);
29-
}
29+
const trustedItems = flattenUrls.filter(({ type }) =>
30+
['trusted-url'].includes(type),
31+
);
3032

31-
function matchContainsOnlyRegistyMatches(match) {
32-
return match.urls.every(url => url.type === 'registry');
33+
return {
34+
apiItems,
35+
internalItems,
36+
trustedItems,
37+
};
3338
}
3439

35-
function filterLiveResolver(matches) {
36-
return matches.reduce((memo, match) => {
37-
if (matchContainsOnlyRegistyMatches(match)) {
38-
memo.push(match);
40+
function insertLinks({
41+
matches,
42+
githubTree,
43+
octolinkerApiResponse,
44+
user,
45+
repo,
46+
branch,
47+
}) {
48+
matches.forEach(({ urls, link }) => {
49+
if (link.href) {
50+
// Return early if link is already set
51+
return;
3952
}
4053

41-
return memo;
42-
}, []);
54+
for (const item of urls) {
55+
if (item.type === 'internal-link') {
56+
if (githubTree.includes(item.path)) {
57+
link.href = item.url;
58+
break;
59+
}
60+
} else if (item.type === 'github-search') {
61+
const allMatches = githubTree.filter(path =>
62+
path.endsWith(item.target),
63+
);
64+
65+
if (allMatches.length === 1) {
66+
link.href = `https://github.com/${user}/${repo}/blob/${branch}/${
67+
allMatches[0]
68+
}`;
69+
break;
70+
}
71+
// TODO implement https://www.npmjs.com/package/fast-levenshtein
72+
} else if (['registry', 'ping'].includes(item.type)) {
73+
const finalUrl = octolinkerApiResponse.find(
74+
({ type, target }) =>
75+
(type === item.registry || type === item.type) &&
76+
target === item.target,
77+
);
78+
79+
if (finalUrl && finalUrl.result) {
80+
link.href = finalUrl.result;
81+
break;
82+
}
83+
}
84+
}
85+
});
4386
}
4487

45-
export default function(matches) {
46-
const registryMatch = filterLiveResolver(matches);
47-
runLiveQuery(registryMatch);
88+
export default async function(matches) {
89+
const { apiItems, internalItems, trustedItems } = groupMatchesByType(matches);
90+
91+
let octolinkerApiResponsePromise = [];
92+
let githubTreePromise = [];
93+
94+
if (apiItems.length) {
95+
octolinkerApiResponsePromise = bulkAction(apiItems);
96+
}
97+
98+
let user;
99+
let repo;
100+
let branch;
101+
if (internalItems.length) {
102+
({ user, repo, branch } = getRepoMetadata(internalItems));
103+
104+
if (user && repo && branch) {
105+
githubTreePromise = fetchTree({ user, repo, branch });
106+
}
107+
}
108+
109+
trustedItems.forEach(item => {
110+
item.link.href = item.target;
111+
});
112+
113+
const octolinkerApiResponse = await octolinkerApiResponsePromise;
114+
const githubTree = await githubTreePromise;
115+
116+
// TODO explore resolving on the fly so we don't have to wait for both calls to finish
117+
118+
insertLinks({
119+
matches,
120+
octolinkerApiResponse,
121+
githubTree,
122+
user,
123+
repo,
124+
branch,
125+
});
48126
}

0 commit comments

Comments
 (0)