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.

Sign up for GitHub

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

refactor(isExternal): avoid undefined & improve performance #124

Merged
merged 6 commits into from
Nov 17, 2019
Merged

refactor(isExternal): avoid undefined & improve performance #124

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
623ba26
refactor(isExternal): avoid undefined & improve performance
SukkaW Nov 8, 2019
b7e4d9f
refactor(isExternal): update
SukkaW Nov 9, 2019
32bef13
refactor(isExternal): rename url to input
SukkaW Nov 9, 2019
d2c01a7
docs(isExternalLink): "url" as parameter name
curbengh Nov 9, 2019
3408824
fix(isExternalLink): handle when there is no external_link or exclude
SukkaW Nov 9, 2019
6ea2fb7
style: remove extra new line
curbengh Nov 17, 2019
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
22 changes: 8 additions & 14 deletions lib/is_external_link.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
'use strict';

const { URL } = require('url');

const urlObj = (str) => {
try {
return new URL(str);
} catch (err) {
return str;
}
};
const { parse, URL } = require('url');

/**
* Check whether the link is external
Expand All @@ -18,17 +10,19 @@ const urlObj = (str) => {

function isExternalLink(url) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest to rename to path or input to not confuse with new URL().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

input might be a good idea.

const { config } = this;
const exclude = Array.isArray(config.external_link.exclude) ? config.external_link.exclude
: [config.external_link.exclude];
const data = urlObj(url);
const host = data.hostname;
const sitehost = typeof urlObj(config.url) === 'object' ? urlObj(config.url).hostname : config.url;
const sitehost = parse(config.url).hostname || config.url;
// Pass a base to new URL
Copy link
Contributor

@curbengh curbengh Nov 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say the purpose is // Relative url handling.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to // handle relative url since there is a handle mailto: javascript: vbscript: and so on below.

const data = new URL(url, `http://${sitehost}`);

if (!sitehost || typeof data === 'string') return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with L15, typeof data is no longer required since it will always be an "object".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.


// handle mailto: javascript: vbscript: and so on
if (data.origin === 'null') return false;

const host = data.hostname;
const exclude = Array.isArray(config.external_link.exclude) ? config.external_link.exclude
: [config.external_link.exclude];

if (exclude && exclude.length) {
for (const i of exclude) {
if (host === i) return false;
Expand Down