Skip to content

WIP, updating server.js to use JSDOM to fix isExternal, but also some… #3

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

Closed
wants to merge 7 commits into from
Closed

WIP, updating server.js to use JSDOM to fix isExternal, but also some… #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0a8deed
feat: Qualify URLs then see if the origins match.
trusktr Apr 26, 2020
ed08428
feat: also update the client isExternal check
trusktr Apr 26, 2020
bbeaafd
fix: fix lint errors
trusktr Apr 26, 2020
8dd273b
fix: automatically fix some lint warnings
trusktr Apr 27, 2020
2bc8253
use the more robust and shared isExternal function
trusktr May 21, 2020
22f1b2b
add an intial test file for testing SSR
trusktr May 21, 2020
f18b994
WIP compiler.parse() is called without arg, causes SSR mode to not wo…
trusktr Jun 17, 2020
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
43 changes: 22 additions & 21 deletions build/ssr.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,35 +1,36 @@
var rollup = require('rollup')
var buble = require('rollup-plugin-buble')
var async = require('rollup-plugin-async')
var replace = require('rollup-plugin-replace')
var rollup = require('rollup');
// var buble = require('rollup-plugin-buble');
// var async = require('rollup-plugin-async')
var replace = require('rollup-plugin-replace');

rollup
.rollup({
input: 'packages/docsify-server-renderer/index.js',
plugins: [
async(),
// async(),
replace({
__VERSION__: process.env.VERSION || require('../package.json').version,
'process.env.SSR': true
'process.env.SSR': true,
}),
buble({
transforms: {
generator: false
}
})
// TODO restore this, for IE11.
// buble({
// transforms: {
// generator: false,
// },
// }),
Copy link
Author

Choose a reason for hiding this comment

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

The output is nice without this. async functions and generators work great in newer Node.

],
onwarn: function () {}
onwarn: function() {},
})
.then(function (bundle) {
var dest = 'packages/docsify-server-renderer/build.js'
.then(function(bundle) {
var dest = 'packages/docsify-server-renderer/build.js';

console.log(dest)
console.log(dest);
Copy link
Author

Choose a reason for hiding this comment

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

auto formatting based on the repo's eslint/prettier config files.

return bundle.write({
format: 'cjs',
file: dest
})
})
.catch(function (err) {
console.error(err)
process.exit(1)
file: dest,
});
})
.catch(function(err) {
console.error(err);
process.exit(1);
});
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
"pub": "sh build/release.sh",
"postinstall": "opencollective-postinstall"
},
"husky": {
"husky-OFF": {
Copy link
Author

Choose a reason for hiding this comment

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

temporary. if we accept any changes, they should all be formatted.

Actually we should just go ahead and format the whole code base all in one PR and get it out of the way if we're going to have the eslint/prettier configs in the repo.

"hooks": {
"pre-commit": "lint-staged"
}
Expand Down
17 changes: 17 additions & 0 deletions packages/docsify-server-renderer/default-template.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<title>docsify</title>
<meta
name="viewport"
content="width=device-width, user-scalable=no, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0"
/>
<link rel="stylesheet" href="/themes/vue.css" title="vue" />
</head>
<body>
<!--inject-app-->
<!--inject-config-->
<script src="/lib/docsify.js"></script>
</body>
</html>
8 changes: 8 additions & 0 deletions packages/docsify-server-renderer/default-template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import fs from 'fs';
import path from 'path';

const tmplPath = path.resolve(__dirname, 'default-template.html');

export function getDefaultTemplate() {
return fs.readFileSync(tmplPath).toString();
}
45 changes: 20 additions & 25 deletions packages/docsify-server-renderer/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,35 +9,29 @@ import { Compiler } from '../../src/core/render/compiler';
import { isAbsolutePath } from '../../src/core/router/util';
import * as tpl from '../../src/core/render/tpl';
import { prerenderEmbed } from '../../src/core/render/embed';
import { getDefaultTemplate } from './default-template';

export { getDefaultTemplate };

function cwd(...args) {
return resolve(process.cwd(), ...args);
}

// Borrowed from https://j11y.io/snippets/getting-a-fully-qualified-url.
function qualifyURL(url) {
// TODO this doesn't work in Node, passing in / results in /. It doesn't know the origin. Maybe we should update `location` globally first.
Copy link
Author

Choose a reason for hiding this comment

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

nevermind this comment. I fixed it by configuring JSDom.

const img = document.createElement('img');
img.src = url; // set string url
url = img.src; // get qualified url
img.src = ''; // prevent the server request
return url;
}

function isExternal(url) {
let match = url.match(
/^([^:\/?#]+:)?(?:\/\/([^\/?#]*))?([^?#]+)?(\?[^#]*)?(#.*)?/
);
if (
typeof match[1] === 'string' &&
match[1].length > 0 &&
match[1].toLowerCase() !== location.protocol
) {
return true;
}
if (
typeof match[2] === 'string' &&
match[2].length > 0 &&
match[2].replace(
new RegExp(
':(' + { 'http:': 80, 'https:': 443 }[location.protocol] + ')?$'
),
''
) !== location.host
) {
return true;
}
return false;
url = qualifyURL(url);
// console.log('qualified URL:', url, location.origin);
url = new URL(url);
return url.origin !== location.origin;
}

function mainTpl(config) {
Expand All @@ -59,12 +53,11 @@ function mainTpl(config) {
}

export default class Renderer {
constructor({ template, config, cache }) {
constructor({ template, config }) {
this.html = template;
this.config = config = Object.assign({}, config, {
routerMode: 'history',
});
this.cache = cache;

this.router = new AbstractHistory(config);
this.compiler = new Compiler(config, this.router);
Expand Down Expand Up @@ -208,4 +201,6 @@ export default class Renderer {
}
}

export { Renderer };

Renderer.version = '__VERSION__';
58 changes: 29 additions & 29 deletions server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
const liveServer = require('live-server')
const isSSR = !!process.env.SSR
const middleware = []
const liveServer = require('live-server');
const isSSR = !!process.env.SSR;
const middleware = [];

if (isSSR) {
const Renderer = require('./packages/docsify-server-renderer/build.js')
const { initJSDOM } = require('./test/_helper');

const dom = initJSDOM('', {
url: 'https://127.0.0.1:3000',
});

require = require('esm')(module /* , options */);

const {
Renderer,
getDefaultTemplate,
} = require('./packages/docsify-server-renderer/index');
Copy link
Author

@trusktr trusktr May 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Import the ES Modules. Much nicer experience in the debugger (Chrome devtools).

Try it:

npm i -g ndb
SSR=true ndb server.js

Once you run it, you should see an error inside the compiler.path() method. Pause devtools on uncaught exceptions in the Sources tab.


debugger;
Copy link
Author

Choose a reason for hiding this comment

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

temporary. I will delete this commit and recreate the branch all cleaned up. We just need to decide what to do.


const renderer = new Renderer({
template: `
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>docsify</title>
<meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0">
<link rel="stylesheet" href="/themes/vue.css" title="vue">
</head>
<body>
<!--inject-app-->
<!--inject-config-->
<script src="/lib/docsify.js"></script>
</body>
</html>`,
Copy link
Author

@trusktr trusktr May 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

moved to a separate HTML file so it can be re-used (f.e. here, and in the test code).

template: getDefaultTemplate(),
config: {
name: 'docsify',
repo: 'docsifyjs/docsify',
Expand All @@ -32,24 +32,24 @@ if (isSSR) {
'/de-de/changelog': '/changelog',
'/zh-cn/changelog': '/changelog',
'/changelog':
'https://raw.githubusercontent.com/docsifyjs/docsify/master/CHANGELOG'
}
'https://raw.githubusercontent.com/docsifyjs/docsify/master/CHANGELOG',
},
},
path: './'
})
// path: './', // not used for anything?
Copy link
Author

@trusktr trusktr May 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Unused? The Renderer constructor doesn't use this option for anything, unless I missed it.

Copy link
Author

@trusktr trusktr May 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, I found out because I add // @ts-check at the top of any JS file and have TypeScript do live type checking in VS Code, and it complained that the option was unused (which appears to be the case).

Looks like we have some cleanup to do.

});

middleware.push(function(req, res, next) {
if (/\.(css|js)$/.test(req.url)) {
return next()
return next();
}
renderer.renderToString(req.url).then(html => res.end(html))
})
renderer.renderToString(req.url).then(html => res.end(html));
});
}

const params = {
port: 3000,
watch: ['lib', 'docs', 'themes'],
middleware
}
middleware,
};

liveServer.start(params)
liveServer.start(params);
37 changes: 13 additions & 24 deletions src/core/fetch/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,19 @@ function loadNested(path, qs, file, next, vm, first) {
).then(next, _ => loadNested(path, qs, file, next, vm));
}

function isExternal(url) {
let match = url.match(
/^([^:\/?#]+:)?(?:\/\/([^\/?#]*))?([^?#]+)?(\?[^#]*)?(#.*)?/
);
if (
typeof match[1] === 'string' &&
match[1].length > 0 &&
match[1].toLowerCase() !== location.protocol
) {
return true;
}
if (
typeof match[2] === 'string' &&
match[2].length > 0 &&
match[2].replace(
new RegExp(
':(' + { 'http:': 80, 'https:': 443 }[location.protocol] + ')?$'
),
''
) !== location.host
) {
return true;
}
return false;
// Borrowed from https://j11y.io/snippets/getting-a-fully-qualified-url.
function qualifyURL(url) {
const img = document.createElement('img');
img.src = url; // set string url
url = img.src; // get qualified url
img.src = ''; // prevent the server request
return url;
}

export function isExternal(url) {
url = qualifyURL(url);
url = new URL(url);
return url.origin !== location.origin;
}

export function fetchMixin(proto) {
Expand Down
1 change: 1 addition & 0 deletions src/core/render/compiler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export class Compiler {
return html;
})(text);

// TODO parse() expects an arg, but here it does not receive an arg so it fails.
const curFileName = this.router.parse().file;

if (isCached) {
Expand Down
6 changes: 3 additions & 3 deletions src/core/render/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable no-unused-vars */
import tinydate from 'tinydate';
import DOMPurify from 'dompurify';
import tinydate from 'tinydate';
import * as dom from '../util/dom';
import cssVars from '../util/polyfill/css-vars';
import { callHook } from '../init/lifecycle';
Expand All @@ -9,6 +9,7 @@ import { getPath, isAbsolutePath } from '../router/util';
import { isMobile, inBrowser } from '../util/env';
import { isPrimitive } from '../util/core';
import { scrollActiveSidebar } from '../event/scroll';
import { isExternal } from '../fetch';
import { Compiler } from './compiler';
import * as tpl from './tpl';
import { prerenderEmbed } from './embed';
Expand Down Expand Up @@ -259,10 +260,9 @@ export function initRender(vm) {

if (config.logo) {
const isBase64 = /^data:image/.test(config.logo);
const isExternal = /(?:http[s]?:)?\/\//.test(config.logo);
const isRelative = /^\./.test(config.logo);

if (!isBase64 && !isExternal && !isRelative) {
if (!isBase64 && !isExternal(config.logo) && !isRelative) {
config.logo = getPath(vm.router.getBasePath(), config.logo);
}
}
Expand Down
10 changes: 8 additions & 2 deletions test/_helper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,14 @@ function ready(callback) {

module.exports.initJSDOM = initJSDOM;

/** @param {string} markup - The HTML document to initialize JSDOM with. */
function initJSDOM(markup, options = {}) {
/**
* Creates a JSDOM instance and assigns the following variables to Node's
* `global`: window, document, navigator, location, XMLHttpRequest.
*
* @param {string} markup - The HTML document to initialize JSDOM with.
* @param {object} options - Options to pass to JSDOM. See https://github.com/jsdom/jsdom#customizing-jsdom
*/
function initJSDOM(markup = '', options = {}) {
const dom = new JSDOM(markup, options);

global.window = dom.window;
Expand Down
1 change: 0 additions & 1 deletion test/unit/docsify.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ describe('Docsify public API', () => {
});

it('global APIs are available', async () => {
// const DOM = new (require('jsdom').JSDOM)(markup, {
const DOM = initJSDOM(markup, {
url: docsifySite,
runScripts: 'dangerously',
Expand Down
44 changes: 44 additions & 0 deletions test/unit/server.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// @ts-check
/* eslint-disable no-global-assign */
require = require('esm')(
module /* , options */
); /* eslint-disable-line no-global-assign */
const { expect } = require('chai');
const { initJSDOM } = require('../_helper');

// const port = 9754;
// const docsifySite = 'http://127.0.0.1:' + port;

initJSDOM();

const {
Renderer,
getDefaultTemplate,
} = require('../../packages/docsify-server-renderer/index');

describe('pacakges/docsify-server-render', function() {
it('renders content', async function() {
const renderer = new Renderer({
template: getDefaultTemplate(),
config: {
name: 'docsify',
repo: 'docsifyjs/docsify',
// basePath: 'https://docsify.js.org/',
loadNavbar: true,
loadSidebar: true,
subMaxLevel: 3,
auto2top: true,
alias: {
'/de-de/changelog': '/changelog',
'/zh-cn/changelog': '/changelog',
'/changelog':
'https://raw.githubusercontent.com/docsifyjs/docsify/master/CHANGELOG',
},
},
});

await renderer.renderToString('/changelog');

expect(renderer).to.be.an.instanceof(Renderer);
Copy link
Author

Choose a reason for hiding this comment

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

A starting point to officially test the server code.

});
});