Add script for checking lit.dev redirects #468
Conversation
aomarks
requested review from
justinfagnani,
arthurevans and
AndrewJakubowicz
|
A live preview of this PR will be available at the URL(s) below. https://pr468-65e12fc---lit-dev-5ftespv5na-uc.a.run.app/ |
|
Actually I'm going to create an ESM package for this, will reopen when ready. |
The lit-dev-tools package is currently CommonJS, because mostly it is used for Eleventy plugins, and Eleventy doesn't support ES modules (11ty/eleventy#836). We want ES modules for this new redirect checker script, because it needs to import some ES modules, and that is difficult to do with TypeScript, because TypeScript doesn't allow emitting an actual `import` statement, which is how CommonJS -> ESM interop works (microsoft/TypeScript#43329). We also an't really have a mix of CommonJS and ESM in the same package, because the {"type": "module"} field has to be set. We could use .mjs extensions, but TypeScript won't emit those. So the simplest solution seems to be to just have two packages!
|
Ready for review again. |
aomarks
force-pushed
the
redirect-checker
branch
from
65e12fc to
56cda15
Compare
aomarks
force-pushed
the
redirect-checker
branch
from
56cda15 to
24e8ba2
Compare
| const checkRedirect = async ( | ||
| redirect: string | ||
| ): Promise<ErrorMessage | typeof OK> => { | ||
| if (isUrl(redirect)) { |
There was a problem hiding this comment.
Non action thinking out loud: I initially thought "a relative url is a url" so had some confusion, since we are differentiating between absolute URLs and relative URLs with this check.
Comment on L46 helped.
There was a problem hiding this comment.
True, that is a little confusing. Renamed to isAbsoluteUrl.
Custom script for checking lit.dev redirects.
It would be nice if we could use the 3rd party link checker we already have for this somehow, but it doesn't support checking for anchors (see stevenvachon/broken-link-checker#108 -- understandable since it would require DOM parsing) which is one of the main failure cases.
Fixes #467 (since we shouldn't need comments if we have the redirects checked in CI).
As part of this, I created a new
lit-dev-tools-esmpackage.The existing
lit-dev-toolspackage is currently CommonJS, because mostly it is used for Eleventy plugins, and Eleventy doesn't support ES modules (11ty/eleventy#836).We want ES modules for this new redirect checker script, because it needs to import some ES modules, and that is difficult to do with TypeScript, because TypeScript doesn't allow emitting an actual
importstatement, which is how CommonJS -> ESM interop works (microsoft/TypeScript#43329).We also can't really have a mix of CommonJS and ESM in the same package, because the
{"type": "module"}field has to be set to one or the other in thepackage.json. We could use.mjsextensions, but TypeScript won't emit those.So the simplest solution seems to be to just have two packages!