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

fix: only log available bindings once in dev #1163

Merged
merged 1 commit into from
Jun 2, 2022
Merged

Conversation

threepointone
Copy link
Contributor

@threepointone threepointone commented Jun 1, 2022

Because we were calling printBindings during the render phase of <Dev/>, we were logging the bindings multiple times (render can be called multiple times, and the interaction of Ink's stdout output intermingled with console is a bit weird). We could have put it into an effect, but I think a better solution here is to simply log it before we even start rendering <Dev/> (so we could see the bindings even if Dev fails to load, for example).

This also adds a fix that masks any overriden values so that we don't accidentally log potential secrets into the terminal.

@changeset-bot
Copy link

changeset-bot bot commented Jun 1, 2022

🦋 Changeset detected

Latest commit: d7a5bc2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
wrangler Patch

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

@github-actions
Copy link
Contributor

github-actions bot commented Jun 1, 2022

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.developers.workers.dev/runs/2421843892/npm-package-wrangler-1163

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/1163/npm-package-wrangler-1163

Or you can use npx with this latest build directly:

npx https://prerelease-registry.developers.workers.dev/runs/2421843892/npm-package-wrangler-1163 dev path/to/script.js

@threepointone
Copy link
Contributor Author

Oops I should add a test for the masking

Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

+1 for adding var-masking tests.

packages/wrangler/src/index.tsx Outdated Show resolved Hide resolved
Because we were calling `printBindings` during the render phase of `<Dev/>`, we were logging the bindings multiple times (render can be called multiple times, and the interaction of Ink's stdout output intermingled with console is a bit weird). We could have put it into an effect, but I think a better solution here is to simply log it before we even start rendering `<Dev/>` (so we could see the bindings even if Dev fails to load, for example).

This also adds a fix that masks any overriden values so that we don't accidentally log potential secrets into the terminal.
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.

2 participants