linkinator icon indicating copy to clipboard operation
linkinator copied to clipboard

urlRewriteSearch and urlRewriteExpressions options don't seem to be implemented

Open holly-cummins opened this issue 3 years ago • 2 comments

I was having some problems getting urlRewriteExpressions to work. I tried urlRewriteSearch and urlRewriteReplace instead, but it also wasn't doing anything. I assumed I was specifying the regex wrong in some way, so I went to look at the source to try and figure out what I was supposed to pass through.

Searching the code, I can see typescript interfaces and config definitions for the options, but I can't see anything in index.ts (or anywhere else) that actually does something with the options. If this is right, I guess either an implementation needs to be added or the options need to be removed from README.MD. I can't see any issues for this, though, so either the features are really unpopular, or I'm missing something!

holly-cummins avatar Apr 20 '22 12:04 holly-cummins

Greetings! This is one of those weird things where the CLI and API accept different parameters. The CLI accepts --url-rewrite-search and --url-rewrite-replace, but those are converted into an array of urlRewriteExpressions inside of cli.ts: https://github.com/JustinBeckwith/linkinator/blob/main/src/cli.ts#L197

The patterns in that array are applied here: https://github.com/JustinBeckwith/linkinator/blob/main/src/index.ts#L164

And gently tested here 🙃 https://github.com/JustinBeckwith/linkinator/blob/main/test/test.index.ts#L542

Could I trouble you for a basic reproduction steps? Are you trying to use these from the CLI, the API, the GitHub Action, or a config file? Details here would be super, super helpful!

JustinBeckwith avatar May 10 '22 18:05 JustinBeckwith

I wish I’d kept better notes about the code I was looking at where I didn’t find an implementation. Perhaps I looked only in index.ts for urlRewriteSearch, and then when I decided it should be urlRewriteExpressions I didn’t read the code carefully enough to see what the syntax was. The cli and api options being different makes what I saw make sense.

I’d tried the following:

return await checker.check({
    path,
    recurse: true,
    linksToSkip,
    urlRewriteSearch: /http:\/\/duckydevine.com/,
    urlRewriteReplace: "http://localhost:9000",
    urlRewriteExpressions: [/http:\/\/duckydevine.com/, "http://localhost:9000"], // Not working
    concurrency: 100, // The twitter URLs seem to work better with a high concurrency, counter-intuitively
    timeout: 30 * 1000
  });
});

Originally I didn’t have both search/replace and expressions, but I was getting desperate. :)

I can see from the code you linked I got the format for urlRewriteExpressions totally wrong, and there should be an inner object in that array.

return await checker.check({
  path,
  recurse: true,
  linksToSkip,
  urlRewriteExpressions: [
    { pattern: /http:\/\/duckydevine.com/, replacement: "http://localhost:9000" }
  ],
  concurrency: 100, // The twitter URLs seem to work better with a high concurrency, counter-intuitively
  timeout: 30 * 1000
});

The json format is not obvious from the readme,

urlRewriteExpressions (array) - Collection of objects that contain a search pattern, and replacement.

So maybe an example of urlRewriteExpressions usage in the readme would be useful. But I’ve got it working on my end, so thank you!

holly-cummins avatar Sep 09 '22 16:09 holly-cummins