Skip to content

All my changes (don't merge) #1091

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 5 commits into from
Closed

All my changes (don't merge) #1091

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bedd47a
WIP prevent scripts from being executed twice
trusktr Aug 25, 2019
fc5a937
more fixes
trusktr Sep 21, 2019
91ea8f2
remove debuggers
trusktr Jan 2, 2020
ae01327
Merge branch 'prepare-script' into fixes
trusktr Jan 25, 2020
01f00b1
convert prepare script to prepack script, which also runs on git inst…
trusktr Feb 16, 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
3 changes: 0 additions & 3 deletions .vscode/settings.json
View file
Open in desktop

This file was deleted.

41 changes: 30 additions & 11 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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 @@ -37,7 +37,7 @@
"build:ssr": "node build/ssr.js",
"build:cover": "node build/cover.js",
"build": "rimraf lib themes/* && run-s build:js build:css build:css:min build:ssr build:cover",
"prepare": "npm run build",
"prepack": "npm run build",
"pub:next": "cross-env RELEASE_TAG=next sh build/release.sh",
"pub": "sh build/release.sh",
"postinstall": "opencollective postinstall"
Expand Down
4 changes: 2 additions & 2 deletions src/core/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {merge, hyphenate, isPrimitive, hasOwn} from './util/core'

export default function () {
export default function (vm) {
const config = merge(
{
el: '#app',
Expand Down Expand Up @@ -28,7 +28,7 @@ export default function () {
noCompileLinks: [],
relativePath: false
},
window.$docsify
typeof window.$docsify === 'function' ? window.$docsify(vm) : window.$docsify
)

const script =
Expand Down
2 changes: 1 addition & 1 deletion src/core/init/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {isFn} from '../util/core'
export function initMixin(proto) {
proto._init = function () {
const vm = this
vm.config = config()
vm.config = config(vm)

initLifecycle(vm) // Init hooks
initPlugin(vm) // Install plugins
Expand Down
14 changes: 10 additions & 4 deletions src/core/render/compiler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,16 @@ export class Compiler {
!_self._matchNotCompileLink(href) &&
!config.ignore
) {
if (href === _self.config.homepage) {
href = 'README'
// skip hrefs like `#/page?id=section`, which are already in the format
// Docsify compiles hrefs to
// TODO move this to router.toURL
// TODO make a note of why we need this
if (!href.trim().startsWith('#/')) {
if (href === _self.config.homepage) {
href = 'README'
}
href = router.toURL(href, null, router.getCurrentPath())
Copy link
Member Author

Choose a reason for hiding this comment

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

As the comment says, this prevents Docsify from erroneously handling links that are already in Docsify's format. This makes it easy to link to known pages if you already know the URL you want to link to. I think this is related to the next comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't remember why I needed this. I'll report back after some investigation.

}
href = router.toURL(href, null, router.getCurrentPath())
} else {
attrs += href.indexOf('mailto:') === 0 ? '' : ` target="${linkTarget}"`
}
Expand All @@ -258,7 +264,7 @@ export class Compiler {
attrs += ` title="${title}"`
}

return `<a href="${href}"${attrs}>${text}</a>`
return `<a docsify-link href="${href}"${attrs}>${text}</a>`
Copy link
Member Author

Choose a reason for hiding this comment

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

I needed a way in my own code to detect Docsify links. The reason is that I have a custom html() hook to modify the markdown rendering and needed to skip links that are already handled by Docisfy:

This is my html hook:

			markdown: {
				// `this` in the following hooks is an instace of Marked Renderer
				renderer: {
					html(html) {
						const matches = html.matchAll(reAnchorWithHref)

						const {linkTarget, router} = vm.compiler

						// if we find an anchor tag with an href attribute
						for (const match of matches) {
							// if the link is a Docsify link generated from markdown, skip it, it is already handled
							if (match[0].startsWith('<a docsify-link')) continue // <--------------------- HERE

							// the result will be one of the three capturing groups from the regex
							let href = match[1] || match[2] || match[3]

							const originalHref = href

							// the first two capturing groups catch single or double quoted values
							const hasQuotes = !!(match[1] || match[2])

							// based on Docsify's Compiler._initRenderer() logic for the markdown "link" hook {{{

							// TODO make some syntax for telling it to ignore the compiling the href
							const ignoreLink = false

							if (
								!Docsify.util.isAbsolutePath(href) &&
								!vm.compiler._matchNotCompileLink(href) &&
								!ignoreLink &&
								// skip hrefs like `#/page?id=section`, which
								// are already in the format Docsify compiles
								// hrefs to
								// TODO move this to router.toURL
								!href.trim().startsWith('#/')
							) {
								if (href === vm.compiler.config.homepage) {
									href = 'README'
								}
								href = router.toURL(href, null, router.getCurrentPath())
							}

							// }}}

							if (!hasQuotes) href = '"' + href + '"'

							html = html.replace(originalHref, href)
						}

						return html
					},

I need to think about if this is the best way to do what I wanted, but I don't remember what problem I needed to solve. I'll need to disable this and see how it breaks my links so I can remember. 😄

Some of the logic is duplicated from other code inside Docsify, which isn't ideal. Let me think of how it can be easier...

Copy link
Member

Choose a reason for hiding this comment

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

for, this I think we can have a use case if we reshape this a little.

like mentioned here in this issue #1088 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

Note also this piece of code is where I needed the vm instance that was passed into the $docsify config when it is a function.

like mentioned here in this issue #1088 (comment)

Gotcha, so if it had that feature then I could detect the ID prefix, similar to what I did with the docsify-link attribute.

}
origin.paragraph = renderer.paragraph = function (text) {
let result
Expand Down
24 changes: 18 additions & 6 deletions src/core/render/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,29 @@ import {isPrimitive} from '../util/core'
import {scrollActiveSidebar, scroll2Top} from '../event/scroll'
import {prerenderEmbed} from './embed'

function executeScript() {
function getScript() {
const script = dom
.findAll('.markdown-section>script')
.filter(s => !/template/.test(s.type))[0]

if (!script) {
return false
return null
}

const code = script.innerText.trim()

if (!code) {
return false
return null
}

return code
}

function executeScript() {
const code = getScript()

if (!code) return

setTimeout(_ => {
window.__EXECUTE_RESULT__ = new Function(code)()
}, 0)
Expand Down Expand Up @@ -52,13 +63,14 @@ function renderMain(html) {
if (
this.config.executeScript !== false &&
typeof window.Vue !== 'undefined' &&
!executeScript()
// !executeScript()
!getScript()
) {
setTimeout(_ => {
// setTimeout(_ => {
const vueVM = window.__EXECUTE_RESULT__
vueVM && vueVM.$destroy && vueVM.$destroy()
window.__EXECUTE_RESULT__ = new window.Vue().$mount('#main')
}, 0)
// }, 0)
Copy link
Member Author

Choose a reason for hiding this comment

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

The changes with getScript/executeScript prevents script tags from executing twice. The setTimeout isn't needed, at least as far as I can tell everything works for me fine.

Maybe it was a hack to defer script execution to later so that things in the DOM are ready? If that's the case, users can for example use jQuery.ready() like normal (or etc).

Copy link
Member

Choose a reason for hiding this comment

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

I think it will effect the custom scrip option which are defined in the markdown right ?

Copy link
Member Author

Choose a reason for hiding this comment

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

custom scrip option which are defined in the markdown right

Can you expand on this?

} else {
this.config.executeScript && executeScript()
}
Expand Down