-
Notifications
You must be signed in to change notification settings - Fork 550
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
fix: do not render "value of stdout.lastframe() is undefined" if the output is an empty string #2912
fix: do not render "value of stdout.lastframe() is undefined" if the output is an empty string #2912
Conversation
…output is an empty string Fixes cloudflare#2907
🦋 Changeset detectedLatest commit: 8f383e6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/4446793807/npm-package-wrangler-2912 You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/2912/npm-package-wrangler-2912 Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/4446793807/npm-package-wrangler-2912 dev path/to/script.js Additional artifacts:npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/4446793807/npm-package-cloudflare-pages-shared-2912 Note that these links will no longer work once the GitHub Actions artifact expires. |
Codecov Report
@@ Coverage Diff @@
## main #2912 +/- ##
==========================================
+ Coverage 74.06% 74.09% +0.02%
==========================================
Files 167 167
Lines 10418 10418
Branches 2785 2785
==========================================
+ Hits 7716 7719 +3
+ Misses 2702 2699 -3
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed this fixes the issue via preview build
I don't know if it stops statements excution. I have three slq commands in .sql file but only one statement is excuted.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! We have had turning some things to ?? be a problem glad to see it fix stuff too lol
Fixes #2907
What this PR solves / how to test:
If the components passed to
renderToString()
resulted in an empty string, we were incorrectly converting that to a warning message. This message should only apply if there is a problem with rendering and it returnsundefined
.To test run the following commands:
npx wrangler d1 create TEST_DB npx wrangler d1 execute TEST_DB --command "DROP TABLE IF EXISTS test_table"
You should not see the "value of stdout.lastframe() is undefined" message.
Author has included the following, where applicable:
TestsD1 query execution is not possible to unit test - the WASM causes a segmentation fault in NodeJS.Reviewer has performed the following, where applicable: