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

As of @sveltejs/kit version 1.0.13, crypto doesn't seem to be polyfilled during build-time #8655

Closed
mglikesbikes opened this issue Jan 21, 2023 · 5 comments · Fixed by #8636
Labels
bug Something isn't working

Comments

@mglikesbikes
Copy link

mglikesbikes commented Jan 21, 2023

Describe the bug

Looks like prior to 1.0.13, crypto (the webcrypto global) was polyfilled. Now it doesn't seem to be. (Using adapter-cloudflare-workers if that makes a difference.)

For example: this code will prevent vite build from completing:

if (typeof crypto === 'undefined') throw new Error('this explodes `vite build`')

Reproduction

https://stackblitz.com/edit/sveltejs-kit-template-default-zank98?file=README.md,src%2Froutes%2F%2Bpage.js,package.json&terminal=dev

Steps to repro:

  1. Run npm run build with SK 1.0.12. Note it builds fine.
  2. Run npm i -D @sveltejs/kit@1.0.13
  3. Run npm run build and observe that src/pages.js throws an error because crypto is undefined

Logs

No response

System Info

System:
    OS: macOS 13.1
    CPU: (8) arm64 Apple M1 Pro
    Memory: 1.05 GB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 18.10.0 - /opt/homebrew/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 9.2.0 - /opt/homebrew/bin/npm
  Browsers:
    Chrome: 109.0.5414.87
    Safari: 16.2
  npmPackages:
    @sveltejs/adapter-auto: ^1.0.0 => 1.0.0 
    @sveltejs/adapter-cloudflare-workers: ^1.0.0 => 1.0.0 
    @sveltejs/kit: ^1.0.0 => 1.0.1 
    svelte: ^3.54.0 => 3.55.0 
    vite: ^4.0.0 => 4.0.4```

Severity

blocking an upgrade

Additional Information

Tried to debug this as best I could and it may be related to this PR: #8429

(edited for clarity per findings in comments)

@benmccann
Copy link
Member

You could confirm whether it's that PR by testing with 1.0.3 and 1.0.4 and reporting whether or not they work

I would also be curious if either typeof Deno === 'undefined' or typeof Bun === 'undefined' evaluate to true on Cloudflare

@mglikesbikes
Copy link
Author

Good tip — tracked it down to a version 1.0.13 which was this PR: #8429

Stackblitz is updated as well.

@mglikesbikes mglikesbikes changed the title As of @sveltejs/kit version 1.2.2, crypto doesn't seem to be included (?) As of @sveltejs/kit version 1.0.13, crypto doesn't seem to be polyfilled during build-time Jan 22, 2023
@dummdidumm dummdidumm added the bug Something isn't working label Jan 25, 2023
@dummdidumm
Copy link
Member

Problem is that should_polyfill is checked too late, it should happen in postbuild/index.js already since nodes are already loaded there. Let's wait for #8636 to land since that moves stuff around quite a bit, then fix this.

@JordanShurmer
Copy link

JordanShurmer commented Jan 26, 2023

I brought this up in another issue, but it seems this is the most relevant issue.

I am seeing pretty much the same error described in here, except with the fetch object. Moving from 1.0.12 to 1.0.13 causes our builds to fail during the Prerendering step because fetch is not defined.

In our case, we actually want to disable prerendering entirely. I gather, from the comments on the other issue, that this is not really prerendering, despite what the error message claims. Hopefully the error messages in this phase can be adjusted, as well as fixing the issue itself.

@JordanShurmer
Copy link

The solution we're going with for now came from a Discord post, and some other issues related to prerendering:

We're using the building environment flag to just avoiding the code that was using fetch. Fortunately for us it was only in one place, so not too bad.

import {building} from "$app/environment";

...
if (building) return;
fetch(...);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants