Add additional replace rules for node:* modules to custom unit node loaders.

[javorszky]

Closes: https://github.com/nginx/unit/issues/1013

In that particular issue the compiled nuxt files end up importing the http module as node:http rather than http only. This bypasses unit's custom loader implementation which only check for the http or unit-http modules, and their websocket counterparts.

This changeset adds replace sources for both the node:http and node:websocket import signatures.

[javorszky]

Converted to draft to resolve whether we'd need to check for the https and node:https modules as well.

[javorszky]

see https://nodejs.org/api/https.html

and https://nodejs.org/api/http.html

[ac000]

Just some notes for when this is ready proper...

Perhaps some of the PR message could go into the commit message?

I would also prefer the Closes thing to be in the form

Closes: <https://github.com/nginx/unit/issues/1013>

That makes it useful outside of GH, e.g I can click on it in an xterm and have it open in my browser. And if we ever move off of GH...

See here for an example, including other tags you can use... for example you may want to add a Reported-by tag.

[javorszky]

@ac000 I've updated the commit message with description from the PR, and changed the link on both the PR and the commit message to include the full URI of the github issue.

This is now ready to review / merge.

[callahad]

Hm, does GitHub understand that trailer format for automatically closing issues, or does it get confused by the full URL and angle brackets?

Let me test in another repo real quick.

[ac000]

GH will recognise full URL's, but doesn't recognise it being in <>'s.

[ac000]

To quote https://man7.org/linux/man-pages/man7/uri.7.html

When written, URIs should be placed inside double quotes (e.g., "http://www.kernel.org"), enclosed in angle brackets (e.g., \http://lwn.net/), or placed on a line by themselves. A warning for those who use double-quotes: never move extraneous punctuation (such as the period ending a sentence or the comma in a list) inside a URI, since this will change the value of the URI. Instead, use angle brackets instead, or switch to a quoting system that never includes extraneous characters inside quotation marks. This latter system, called the 'new' or 'logical' quoting system by "Hart's Rules" and the "Oxford Dictionary for Writers and Editors", is preferred practice in Great Britain and in various European languages. Older documents suggested inserting the prefix "URL:" just before the URI, but this form has never caught on.

[ac000]

GH should probably learn to understand that format... lets see where to file such a request...

[javorszky]

I don't think GH is going to entertain a request for this. Out of curiosity, is the angle bracket format used somewhere downstream that gets picked up automatically by some tool?

[ac000]

No.

I used to write them without the brackets until Alex pointed me to the above and suggested we should.

I can live without them if it's a problem.

[callahad]

Yeah, GitHub doesn't recognize them for auto-closing. Full URLs,org/repo#issuenum, repo#issuenum, and #issuenum all work. I'm fine with any of those formats. If we want to specify a preferred format, we should move that to a GitHub Discussion.

[ac000]

Full URLs (without the brackets) so they are not tied to and are usable outside of GH.

Obviously email addresses should still be brackets and and also where we refer to GH users where we don't have an email address for them but use their GH profile link.

[tippexs]

Any update on this PR? If we can complete the review I will add this to 1.32 or do we need to push it further out to resolve the https issue posted in https://github.com/nginx/unit/issues/1025

[alejandro-colomar]

I opened a bug-report ticket regarding the Closes: tag to github. It seems those a private, so I can't link here. :|

[javorszky]

Any update on this PR? If we can complete the review I will add this to 1.32 or do we need to push it further out to resolve the https issue posted in #1025

From my point this PR is done. I'm unsure what else is needed to get this merged.

[ac000]

It's just the Closes tag that needs some adjustment...

And to avoid any ambiguity, it should look like this (for now, if and when GH fix the issue with the <>'s we can start putting them in again)

[You can also loose the trailing '.' from the subject line... so]

EDIT: While you're at it you might as well stick my 'Reviewed-by' on there also...

Add additional replace rules for node:* modules

In that particular issue the compiled nuxt files end up importing the
http module as node:http rather than http only. This bypasses unit's
custom loader implementation which only check for the http or unit-http
modules, and their websocket counterparts.

This changeset adds replace sources for both the node:http and
node:websocket import signatures.

Closes: https://github.com/nginx/unit/issues/1013
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>

[javorszky]

@ac000 rebased on latest master and commit message updated

[callahad]

LGTM. Minimal change, already separately reviewed. Changes requested by @ac000 have been applied. Bypassing branch protections and merging.