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

urlRewriteSearch and urlRewriteExpressions options don't seem to be implemented #390

Open
holly-cummins opened this issue Apr 20, 2022 · 2 comments
Labels
needs more info question Further information is requested

Comments

@holly-cummins
Copy link

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!

@JustinBeckwith
Copy link
Owner

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 JustinBeckwith added question Further information is requested needs more info labels May 10, 2022
holly-cummins added a commit to holly-cummins/gatsby-platform that referenced this issue Sep 9, 2022
@holly-cummins
Copy link
Author

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!

@JustinBeckwith JustinBeckwith reopened this Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs more info question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants