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.

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

chore(v2): fix several lint warnings, add missing types, cleanup #3844

Merged
merged 4 commits into from
Nov 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@ module.exports = postcss.plugin(
const sameProperties =
decl.parent.nodes.filter((n) => n.prop === decl.prop) || [];
const hasImportantProperties = sameProperties.some((p) =>
p.hasOwnProperty('important'),
Object.prototype.hasOwnProperty.call(p, 'important'),
);

const overriddenProperties = hasImportantProperties
? sameProperties.filter((p) => !p.hasOwnProperty('important'))
? sameProperties.filter(
(p) => !Object.prototype.hasOwnProperty.call(p, 'important'),
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

that feels a bit verbose, wonder if we shouldn't just disable this eslint rule instead?

Copy link
Contributor Author

@Simek Simek Nov 30, 2020

Choose a reason for hiding this comment

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

I think it's fair to disable it, as much as leaving it on. The side effects or DDoS issues mentioned on the rule page seems not be related to the way those methods are used in the Docusuaurs code, but in the other hand I think that the slightly more verbose syntax won't hurt anyone, and there are only a few usages of hasOwnProperty in the whole codebase.

: sameProperties.slice(0, -1);

overriddenProperties.map((p) => p.remove());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,22 @@ const {
} = getFileLoaderUtils();

const createJSX = (node, pathUrl) => {
node.type = 'jsx';
node.value = `<img ${node.alt ? `alt={"${escapeHtml(node.alt)}"} ` : ''}${
const jsxNode = node;
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not rename the fn param directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

jsxNode.type = 'jsx';
jsxNode.value = `<img ${node.alt ? `alt={"${escapeHtml(node.alt)}"} ` : ''}${
node.url
? `src={require("${inlineMarkdownImageFileLoader}${pathUrl}").default}`
: ''
}${node.title ? ` title="${escapeHtml(node.title)}"` : ''} />`;

if (node.url) {
delete node.url;
if (jsxNode.url) {
delete jsxNode.url;
}
if (node.alt) {
delete node.alt;
if (jsxNode.alt) {
delete jsxNode.alt;
}
if (node.title) {
delete node.title;
if (jsxNode.title) {
delete jsxNode.title;
}
};

Expand Down
3 changes: 2 additions & 1 deletion packages/docusaurus-migrate/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@docusaurus/migrate",
"version": "2.0.0-alpha.69",
"description": "A cli tool to migrate between different version of docusuarus",
"description": "A cli tool to migrate between different version of Docusuarus",
"main": "lib/index.js",
"license": "MIT",
"engines": {
Expand Down Expand Up @@ -43,6 +43,7 @@
"unist-util-visit": "^2.0.2"
},
"devDependencies": {
"@types/color": "^3.0.1",
"@types/jscodeshift": "^0.7.1"
}
}
40 changes: 20 additions & 20 deletions packages/docusaurus-migrate/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ import glob from 'glob';
import Color from 'color';

import {
VersionOneConfig,
VersionTwoConfig,
ClassicPresetEntries,
SidebarEntries,
VersionOneConfig,
VersionTwoConfig,
} from './types';
import extractMetadata, {shouldQuotifyFrontMatter} from './frontMatter';
import migratePage from './transform';
Expand Down Expand Up @@ -229,9 +229,9 @@ export function createConfigFile({
const homePageId = siteConfig.headerLinks?.filter((value) => value.doc)[0]
.doc;

const customConfigFields: Record<string, any> = {};
const customConfigFields: Record<string, unknown> = {};
// add fields that are unknown to v2 to customConfigFields
Object.keys(siteConfig).forEach((key: any) => {
Object.keys(siteConfig).forEach((key) => {
const knownFields = [
'title',
'tagline',
Expand Down Expand Up @@ -289,7 +289,7 @@ export function createConfigFile({
v2DocsPath = path.relative(newDir, absoluteDocsPath);
}

const result: VersionTwoConfig = {
return {
title: siteConfig.title ?? '',
tagline: siteConfig.tagline,
url: siteConfig.url ?? '',
Expand Down Expand Up @@ -330,22 +330,24 @@ export function createConfigFile({
: undefined,
items: (siteConfig.headerLinks ?? [])
.map((link) => {
if (link.doc) {
const {doc, href, label, page} = link;
const position = 'left';
if (doc) {
return {
to: `docs/${link.doc === homePageId ? '' : link.doc}`,
label: link.label,
position: 'left',
to: `docs/${doc === homePageId ? '' : doc}`,
label,
position,
};
}
if (link.page) {
if (page) {
return {
to: `/${link.page}`,
label: link.label,
position: 'left',
to: `/${page}`,
label,
position,
};
}
if (link.href) {
return {href: link.href, label: link.label, position: 'left'};
if (href) {
return {href, label, position};
}
return null;
})
Expand Down Expand Up @@ -379,7 +381,6 @@ export function createConfigFile({
: undefined,
},
};
return result;
}

function createClientRedirects(
Expand Down Expand Up @@ -476,7 +477,7 @@ function handleVersioning(
const loadedVersions: Array<string> = JSON.parse(
String(fs.readFileSync(path.join(siteDir, 'versions.json'))),
);
fs.copyFile(
fs.copyFileSync(
slorber marked this conversation as resolved.
Show resolved Hide resolved
path.join(siteDir, 'versions.json'),
path.join(newDir, 'versions.json'),
);
Expand Down Expand Up @@ -732,11 +733,10 @@ function migrateLatestDocs(
classicPreset: ClassicPresetEntries,
): void {
if (fs.existsSync(path.join(siteDir, '..', 'docs'))) {
const docsPath = path.join(
classicPreset.docs.path = path.join(
path.relative(newDir, path.join(siteDir, '..')),
'docs',
);
classicPreset.docs.path = docsPath;
const files = walk(path.join(siteDir, '..', 'docs'));
files.forEach((file) => {
const content = String(fs.readFileSync(file));
Expand Down Expand Up @@ -797,5 +797,5 @@ export async function migrateMDToMDX(
sanitizedFileContent(String(fs.readFileSync(file)), true),
);
});
console.log(`Succesfully migrated ${siteDir} to ${newDir}`);
console.log(`Successfully migrated ${siteDir} to ${newDir}`);
}
10 changes: 3 additions & 7 deletions packages/docusaurus-migrate/src/sanitizeMD.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ const tags = htmlTags.reduce((acc: {[key: string]: boolean}, tag) => {
}, {});

export default function sanitizeMD(code: string): string {
const markdownTree = unified()
.use(markdown as any)
.parse(code);
const markdownTree = unified().use(markdown).parse(code);
visit(markdownTree, 'code', (node) => {
node.value = `\n<!--${node.value}-->\n`;
});
Expand All @@ -31,12 +29,10 @@ export default function sanitizeMD(code: string): string {
});

const markdownString = unified()
.use(remarkStringify as any, {fence: '`', fences: true})
.use(remarkStringify, {fence: '`', fences: true})
.stringify(markdownTree);

const htmlTree = unified()
.use(parse as any)
.parse(markdownString);
const htmlTree = unified().use(parse).parse(markdownString);
visit(htmlTree, 'element', (node) => {
if (!tags[node.tagName as string]) {
node.type = 'text';
Expand Down
55 changes: 29 additions & 26 deletions packages/docusaurus-migrate/src/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,14 @@
* LICENSE file in the root directory of this source tree.
*/

import jscodeshift from 'jscodeshift';
import jscodeshift, {
ArrowFunctionExpression,
AssignmentExpression,
ASTPath,
Collection,
TemplateElement,
VariableDeclarator,
} from 'jscodeshift';

const empty = () =>
jscodeshift.arrowFunctionExpression(
Expand All @@ -18,77 +25,73 @@ const empty = () =>
),
);

const property = (key: string, value: jscodeshift.ArrowFunctionExpression) =>
const property = (key: string, value: ArrowFunctionExpression) =>
jscodeshift.objectProperty(jscodeshift.identifier(key), value);

const processCallExpression = (
node: jscodeshift.ASTPath<jscodeshift.VariableDeclarator>,
) => {
const processCallExpression = (node: ASTPath<VariableDeclarator>) => {
const args = (node?.value?.init as any)?.arguments[0];
if (args.type === 'Literal') {
if (args.value.includes('../../core/CompLibrary')) {
const newDeclartor = jscodeshift.variableDeclarator(
const newDeclarator = jscodeshift.variableDeclarator(
node.value.id,
jscodeshift.objectExpression([
property('Container', empty()),
property('GridBlock', empty()),
property('MarkdownBlock', empty()),
]),
);
jscodeshift(node).replaceWith(newDeclartor);
jscodeshift(node).replaceWith(newDeclarator);
}
}
if (args.type === 'TemplateLiteral') {
if (
args.quasis
.map((element: jscodeshift.TemplateElement) => element.value.raw)
.map((element: TemplateElement) => element.value.raw)
.join('')
.match(/\/core\//)
) {
const newDeclartor = jscodeshift.variableDeclarator(
const newDeclarator = jscodeshift.variableDeclarator(
node.value.id,
empty(),
);
jscodeshift(node).replaceWith(newDeclartor);
jscodeshift(node).replaceWith(newDeclarator);
}
}
};

const processMemberExpression = (
node: jscodeshift.ASTPath<jscodeshift.VariableDeclarator>,
) => {
const processMemberExpression = (node: ASTPath<VariableDeclarator>) => {
const args = (node?.value?.init as any)?.object?.arguments[0];
if (args.type === 'Literal') {
if (args.value === '../../core/CompLibrary.js') {
const newDeclartor = jscodeshift.variableDeclarator(
const newDeclarator = jscodeshift.variableDeclarator(
node.value.id,
jscodeshift.objectExpression([
property('Container', empty()),
property('GridBlock', empty()),
property('MarkdownBlock', empty()),
]),
);
jscodeshift(node).replaceWith(newDeclartor);
jscodeshift(node).replaceWith(newDeclarator);
} else if (args.value.match(/server/)) {
const newDeclartor = jscodeshift.variableDeclarator(
const newDeclarator = jscodeshift.variableDeclarator(
node.value.id,
empty(),
);
jscodeshift(node).replaceWith(newDeclartor);
jscodeshift(node).replaceWith(newDeclarator);
}
}
if (args.type === 'TemplateLiteral') {
if (
args.quasis
.map((ele: jscodeshift.TemplateElement) => ele.value.raw)
.map((ele: TemplateElement) => ele.value.raw)
.join('')
.match(/\/core\//)
) {
const newDeclartor = jscodeshift.variableDeclarator(
const newDeclarator = jscodeshift.variableDeclarator(
node.value.id,
empty(),
);
jscodeshift(node).replaceWith(newDeclartor);
jscodeshift(node).replaceWith(newDeclarator);
}
}
};
Expand All @@ -113,7 +116,7 @@ export default function transformer(file: string): string {
}

root
.find(jscodeshift.AssignmentExpression, {
.find(AssignmentExpression, {
operator: '=',
left: {
type: 'MemberExpression',
Expand Down Expand Up @@ -164,10 +167,10 @@ export default function transformer(file: string): string {
return root.toSource();
}

function getDefaultImportDeclarators(rootAst: jscodeshift.Collection) {
function getDefaultImportDeclarators(rootAst: Collection) {
// var ... = require('y')
return rootAst
.find(jscodeshift.VariableDeclarator, {
.find(VariableDeclarator, {
init: {
callee: {
name: 'require',
Expand All @@ -179,9 +182,9 @@ function getDefaultImportDeclarators(rootAst: jscodeshift.Collection) {
});
}

function getNamedImportDeclarators(rootAst: jscodeshift.Collection) {
function getNamedImportDeclarators(rootAst: Collection) {
// var ... = require('y').x
return rootAst.find(jscodeshift.VariableDeclarator, {
return rootAst.find(VariableDeclarator, {
init: {
object: {
callee: {
Expand All @@ -192,7 +195,7 @@ function getNamedImportDeclarators(rootAst: jscodeshift.Collection) {
});
}

function getImportDeclaratorPaths(variableDeclaration: jscodeshift.Collection) {
function getImportDeclaratorPaths(variableDeclaration: Collection) {
const defaultImports = getDefaultImportDeclarators(variableDeclaration);

const namedImports = getNamedImportDeclarators(variableDeclaration);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ It is not possible to redirect the same pathname to multiple destinations:
}
},
);
redirects = uniqBy(redirects, (redirect) => redirect.from);
const collectedRedirects = uniqBy(redirects, (redirect) => redirect.from);

// We don't want to override an already existing route with a redirect file!
const redirectsOverridingExistingPath = redirects.filter((redirect) =>
pluginContext.relativeRoutesPaths.includes(redirect.from),
const redirectsOverridingExistingPath = collectedRedirects.filter(
(redirect) => pluginContext.relativeRoutesPaths.includes(redirect.from),
);
if (redirectsOverridingExistingPath.length > 0) {
console.error(
Expand All @@ -100,11 +100,9 @@ It is not possible to redirect the same pathname to multiple destinations:
),
);
}
redirects = redirects.filter(
return collectedRedirects.filter(
(redirect) => !pluginContext.relativeRoutesPaths.includes(redirect.from),
);

return redirects;
}

// For each plugin config option, create the appropriate redirects
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ const getCompiledRedirectPageTemplate = memoize(() => {
return eta.compile(redirectPageTemplate.trim());
});

function renderRedirectPageTemplate(data: object) {
function renderRedirectPageTemplate(data: Record<string, unknown>) {
const compiled = getCompiledRedirectPageTemplate();
return compiled(data, eta.defaultConfig);
}

export default function createRedirectPageContent({
toUrl,
}: CreateRedirectPageOptions) {
}: CreateRedirectPageOptions): string {
return renderRedirectPageTemplate({
toUrl: encodeURI(toUrl),
});
Expand Down
Loading