-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
fix links in docs #4200
fix links in docs #4200
Conversation
build/fix-api-docs.js
Outdated
|
||
var replacements = [ | ||
{find: '\"/docs/guides/\"', replace: '"#"'}, | ||
{find: '\"/docs/guides/(.*)\.md\"', replace: '"tutorial-$1.html"'}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be the first item in the array? That is, do the most specific changes first and work towards more generic ones after?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that probably makes sense let me try it out
build/fix-api-docs.js
Outdated
var replacements = [ | ||
{find: '\"/docs/guides/\"', replace: '"#"'}, | ||
{find: '\"/docs/guides/(.*)\.md\"', replace: '"tutorial-$1.html"'}, | ||
{find: '\"tutorial-tech.html\"', replace: '"tutorial-tech_.html"'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this page rendered as tech_
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure, I think it may have something to do with a file name tech already existing? Either that or the source handler weirdness, (jsdoc links to -_Tech.html
on the Tech API docs)
package.json
Outdated
@@ -27,6 +27,7 @@ | |||
"docs": "npm run docs:lint && npm run docs:api", | |||
"jsdoc": "jsdoc", | |||
"docs:api": "jsdoc -c .jsdoc.json", | |||
"postdocs:api": "node ./build/fix-api-docs.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this runs immediately after docs:api
and before the prepublish
script finishes, as an example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should run directly after docs:api
and before anything else I would think. Would we rather use npm-run-all
here so its more clear?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I think it's fine. Just wanted to verify that prepublish
should work as we expect so the urls will get changed as part of that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
manually verified that this will work by run npm run prepublish
build/fix-api-docs.js
Outdated
var apiPath = path.join(__dirname, '..', 'docs', 'api'); | ||
|
||
var replacements = [ | ||
{find: /\/docs\/guides\/(.+)\.md/, replace: 'tutorial-$1.html'}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these need the global flag to apply properly to all occurrences in the file.
Description
linkdown
to make sure links are workingOutstanding Issues (probably for another PR)
Dom
when they should be usingmodule:dom
Requirements Checklist