Nothing Special   »   [go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

feat: use tough-cookie directly #2396

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

feat: use tough-cookie directly #2396

wants to merge 4 commits into from

Conversation

kettanaito
Copy link
Member
@kettanaito kettanaito commented Dec 16, 2024 •
edited
Loading

This PR aims to drop the wrapper of tough-cookie: https://github.com/bundled-es-modules/tough-cookie
It also upgrades the dep to v5.

The original PR: #2206 used the es-bundled version.
In september with the release of v5 tough-cookie was rewritten in typescript and (I'm assuming here) is probably compatible with ES without this wrapper package.

@kettanaito kettanaito mentioned this pull request Dec 16, 2024
Closed
@kettanaito kettanaito marked this pull request as ready for review December 16, 2024 14:45
@kettanaito
Copy link
Member Author

Build failure

The IIFE build fails because tough-cookie depends on node:url, while IIFE build is meant for the browser:

[IIFE] IIFE Build failed
Error: Build failed with 1 error:
node_modules/.pnpm/tough-cookie@5.0.0/node_modules/tough-cookie/dist/cookie/canonicalDomain.js:5:27: ERROR: Could not resolve "node:url"
    at failureErrorWithLog (/mswjs/msw/node_modules/.pnpm/esbuild@0.24.0/node_modules/esbuild/lib/main.js:1476:15)
    at /mswjs/msw/node_modules/.pnpm/esbuild@0.24.0/node_modules/esbuild/lib/main.js:945:25
    at runOnEndCallbacks (/mswjs/msw/node_modules/.pnpm/esbuild@0.24.0/node_modules/esbuild/lib/main.js:1316:45)
    at buildResponseToResult (/mswjs/msw/node_modules/.pnpm/esbuild@0.24.0/node_modules/esbuild/lib/main.js:943:7)
    at /mswjs/msw/node_modules/.pnpm/esbuild@0.24.0/node_modules/esbuild/lib/main.js:970:16
    at responseCallbacks.<computed> (/mswjs/msw/node_modules/.pnpm/esbuild@0.24.0/node_modules/esbuild/lib/main.js:622:9)
    at handleIncomingPacket (/mswjs/msw/node_modules/.pnpm/esbuild@0.24.0/node_modules/esbuild/lib/main.js:677:12)
    at Socket.readFromStdout (/mswjs/msw/node_modules/.pnpm/esbuild@0.24.0/node_modules/esbuild/lib/main.js:600:7)
    at Socket.emit (node:events:517:28)
    at addChunk (node:internal/streams/readable:368:12)
    at readableAddChunk (node:internal/streams/readable:341:9)
    at Readable.push (node:internal/streams/readable:278:10)
    at Pipe.onStreamRead (node:internal/stream_base_commons:190:23)

Not sure why this isn't the case for @bundled-es-modules/tough-cookie. Maybe it uses an older version of tough-cookie that doesn't have the dependency on node:url?

cc @SamMousa

@kettanaito kettanaito marked this pull request as draft December 16, 2024 14:58
@SamMousa
Copy link

Hmm seems to be the case indeed. They have another closed issue for react native which has similar problem.

There was another implementation of this functionality somewhere, but i can't seem to find the package. It was well maintained for multiple years, I'll give it a look

@kettanaito
Copy link
Member Author

If it's a single dependency, we may try resolving it to something else at build time, like to url, if it's compatible API-wise. It's hacky and I wouldn't do that, but it's a lesser evil than relying on a package that may respect the specification less.

I see no reason for tough-cookie to use node:url, but here we are.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kettanaito @SamMousa