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

Next.js build fails due to this regex #160

Closed
wants to merge 1 commit into from

Conversation

lightyaer
Copy link

@lightyaer lightyaer commented Jan 23, 2024

Next.js build fails when you upgrade to 14.0.3+ versions.

Upon searching I found this issue in next.js repo - vercel/next.js#59540
which points that a regex change which fixes the build issue.

In the latest version of libsql-client-ts 0.4.0-pre.10, the fix has been implemented,
but,

  • some weirdness with string literals and regex parsing OR
  • how next.js build process(rust) parses this lib OR
  • voodoo magic

Fails the build with a Collecting Page Data error.

Using a string concatenation solves this.

Also, tagging another issue from this repo

Fixes #142
Fixes vercel/next.js#59540

Comment on lines 90 to 92
const USERINFO = '(?<username>[^:]*)(:(?<password>.*))?';
const HOST = '((?<host>[^:\\[\\]]*)|(\\[(?<host_br>[^\\[\\]]*)\\]))';
const PORT = '(?<port>[0-9]*)';
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a suggestion here to make this more readable

Suggested change
const USERINFO = '(?<username>[^:]*)(:(?<password>.*))?';
const HOST = '((?<host>[^:\\[\\]]*)|(\\[(?<host_br>[^\\[\\]]*)\\]))';
const PORT = '(?<port>[0-9]*)';
const USERINFO = "((?<username>[^:]*)(:(?<password>.*))?@)?";
const HOST = "((?<host>[^:\\[\\]]*)|(\\[(?<host_br>[^\\[\\]]*)\\]))";
const PORT = "(:(?<port>[0-9]*))?";
return new RegExp("^" + USERINFO + HOST + PORT + "$", "su");

@penberg
Copy link
Contributor

penberg commented Jan 24, 2024

@giovannibenussi @notrab please review

@giovannibenussi
Copy link
Contributor

The code in general looks good but has a few issues that are reflected in the tests. For example, we got the error "URL_PARAM_NOT_SUPPORTED: Unknown URL query parameter \"1\"" in the test below, which means that we're doing something wrong in the parsing. I'll leave further comments in the code if I find something that could be causing those issues and take another look once they're fixed!

test("URL scheme incompatible with ?tls", () => {
    const urls = [
        "ws://localhost?tls=1",
        "wss://localhost?tls=0",
        "http://localhost?tls=1",
        "https://localhost?tls=0",
    ];
    for (const url of urls) {
        expect(() => createClient({url}))
            .toThrow(expect.toBeLibsqlError("URL_INVALID", /TLS/));
    }
});

@penberg
Copy link
Contributor

penberg commented Feb 1, 2024

Fixed by #172 so closing this PR.

@penberg penberg closed this Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build failure for edge runtime with libsql Build error when building it for cloudflare edge runtime
3 participants