nodejs / remark-preset-lint-node

remark preset to configure remark-lint with settings for nodejs/node
https://npm.im/remark-preset-lint-node
MIT License
15 stars 23 forks source link

[Discussion] About special rules for nodejs.org's md files check #195

Open SEWeiTung opened 3 years ago

SEWeiTung commented 3 years ago

Ref Drafts: https://github.com/nodejs/remark-preset-lint-node/pull/194

Considering there're things not decided yet, move here to have a discussion with you:

1) The rules for md files in nodejs.org is quite diff from what we saw before, Maybe this is the ONLY special one in the whole node.js org, so is it possible to define a remarkable link rules to check it? I don't think it good to pollute the original remarkable links' check rules by adding the options, let it be pure, maybe we can create another project with which we make a special thing for nodejs.org?

2) If OK for 1st, then I've finished some of it according to @aduh95 (First give him a BIG THANKS) roughly, and it really can help to find out many error link out, however I cannot create a new project something like "remark-preset-lint-node" due to the authentication & authorization of node.js, maybe we should seperate the special codes from general ones by creating a new project and refer it into nodejs.org?

3) The worst thing for me here is where to get the root disc? If giving the parameter from options outside, it's hard for write it out because the env will be quite different from each other (we cannot hard coded the disc). So until now I get the absolute path with the disc through vfile.cwd, and then combine my absolute links from the current md file together. Of course, some links may have resources such as *.js...ect. I have to suppose they are all under the "static" folder (For more, after running the npm run build, you can see what's on in "build" folder of your folked project locally).

4) At last, share my rough codes (It can run properly with some checks). But not complete, anyone who can create a new project for nodejs.org may use them. You can folk the nodejs.org project and then paste the whole codes to override "node_modules\remark-preset-lint-node\remark-lint-nodejs-links.js", and remove the ["remark-preset-lint-node/remark-lint-nodejs-links", false] in the ".remarkrc".

"use strict";

const fs = require("fs");
const path = require("path");
const { pathToFileURL } = require("url");

const rule = require("unified-lint-rule");

function* getLinksRecursively(node) {
  if (node.url) {
    yield node;
  }
  for (const child of node.children || []) {
    yield* getLinksRecursively(child);
  }
}

function validateLinks(tree, vfile) {
  const currentFileURL = pathToFileURL(path.join(vfile.cwd, vfile.path));
  let previousDefinitionLabel;

  const rootPaths = [pathToFileURL(path.join(vfile.cwd, 'locale'))];

  let targetURL = null;

  for (const node of getLinksRecursively(tree)) {
    if (node.url[0] !== "#") {

      targetURL = new URL(node.url, currentFileURL);

      if (targetURL.protocol === "file:") {

        let possibleFolderPath = 'notExists'

        // If we've checked the link starting with '/', this means
        // we should get its root path and do checks
        if (path.isAbsolute(node.url)) {
          // console.log(`node url is: ${node.url}, ${node.url.endsWith('/')}`);

          // If node.url ends with '/', 
          // This should be a md file under 'locale'
          // or this is a folder with an index.md
          // we are not sure which is right, so try both
          if (node.url.endsWith('/')) {
            const possibleFile = `${node.url.substring(0, node.url.length - 1)}.md`;
            const possibleFolder = `${node.url}index.md`;
            targetURL = new URL(rootPaths[0] + possibleFile);
            possibleFolderPath = new URL(rootPaths[0] + possibleFolder);
          }
          else {
            // Files without "/" should be a resource file under 'static'
            targetURL = new URL(pathToFileURL(vile.cwd) + node.url);
          }
        }
        if (!fs.existsSync(targetURL) && !fs.existsSync(possibleFolderPath)) {
          //console.log(`Target URL not found: ${targetURL} ${possibleFolderPath}`);
          vfile.message("Broken link", node);
        }
      } else if (targetURL.pathname === currentFileURL.pathname) {
        const expected = node.url.includes("#")
          ? node.url.slice(node.url.indexOf("#"))
          : "#";
        vfile.message(
          `Self-reference must start with hash (expected "${expected}", got "${node.url}")`,
          node
        );
      }
    }
    if (node.type === "definition") {
      if (previousDefinitionLabel && previousDefinitionLabel > node.label) {
        vfile.message(
          `Unordered reference ("${node.label}" should be before "${previousDefinitionLabel}")`,
          node
        );
      }
      previousDefinitionLabel = node.label;
    }
  }
}

module.exports = rule("remark-lint:nodejs-links", validateLinks);
aduh95 commented 3 years ago

My two cents on this is an option seems like the easiest way forward to accommodate both nodejs/nodejs.org and nodejs/node (plus other projects that may use this preset). I don't think it would pollute anything as long as the option is not mandatory.

3. The worst thing for me here is where to get the root disc?

What do you mean by root disc? Do you mean the root folder of the hypothetical web server? I think the "only" way would be to force projects to use .remarkrc.js file:

// If ESM syntax:
export default {
  plugins: [
    ["remark-preset-lint-node", {
      resolveAbsolutePathsFrom: new URL('./locale/', import.meta.url),
    }],
  ],
};

// If CJS syntax:
module.exports = {
  plugins: [
    ["remark-preset-lint-node", {
      resolveAbsolutePathsFrom: require('path').join(__dirname, 'locale'),
    }],
  ],
};

Or, considering there are only 29 absolute links in nodejs/nodejs.org, we could also simply add an option to ignore those if there are accepted:

export default {
  plugins: [
    ["remark-preset-lint-node", {
      allowOriginRelativeUrlInLinksAndImages: true,
    }],
  ],
};

Alternatively, a less clean solution would be to disable this rule for the 14 files in nodejs/nodejs.org that contain that kind of links.

wdyt?


          // If node.url ends with '/', 
          // This should be a md file under 'locale'
          // or this is a folder with an index.md
          // we are not sure which is right, so try both
          if (node.url.endsWith('/')) {

FWIW, I don't think it's the correct way forward: node.url could target a directory even if it doesn't end with a /, this plugin is not limited to anchor links, but also images and any node that has a url property... Looking for index.md seems kind of arbitrary, I think checking if the directory exists would be enough to detect the majority of common mistakes.