improve and test vite-plugin-cloudflare `.dev.vars` files support #7864

dario-piotrowicz · 2025-01-22T13:25:01Z

the changes here add tests for making sure that the vite plugin correctly reads secrets from .dev.vars files with and without a specified environment

as part of this wragler's unstable_getMiniflareWorkerOptions utility needed to also be updated to accept the environment name as its second parameter

Tests
- TODO (before merge)
- Tests included (tests added for the vite-plugin, as for wrangler, it looks like the unstable_getMiniflareWorkerOptions utility is not currently tested, so I don't think it's necessary to include any here either)
- Tests not necessary because:
E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
- I don't know
- Required
- Not required because:
Public documentation
- TODO (before merge)
- Cloudflare docs PR(s):
- Documentation not necessary because: documentation added as part of our vite-plugin README (which is currently the only documentation the package has)

changeset-bot · 2025-01-22T13:25:04Z

🦋 Changeset detected

Latest commit: a39093d

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

This PR includes changesets to release 3 packages

Name	Type
@cloudflare/vite-plugin	Patch
wrangler	Minor
@cloudflare/vitest-pool-workers	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 · 2025-01-22T13:39:02Z

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/13001188073/npm-package-wrangler-7864

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

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7864/npm-package-wrangler-7864

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13001188073/npm-package-wrangler-7864 dev path/to/script.js

Additional artifacts:

cloudflare-workers-bindings-extension:

wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13001188073/npm-package-cloudflare-workers-bindings-extension-7864 -O ./cloudflare-workers-bindings-extension.0.0.0-vc2ba65d44.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-vc2ba65d44.vsix

create-cloudflare:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13001188073/npm-package-create-cloudflare-7864 --no-auto-update

@cloudflare/kv-asset-handler:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13001188073/npm-package-cloudflare-kv-asset-handler-7864

miniflare:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13001188073/npm-package-miniflare-7864

@cloudflare/pages-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13001188073/npm-package-cloudflare-pages-shared-7864

@cloudflare/unenv-preset:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13001188073/npm-package-cloudflare-unenv-preset-7864

@cloudflare/vite-plugin:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13001188073/npm-package-cloudflare-vite-plugin-7864

@cloudflare/vitest-pool-workers:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13001188073/npm-package-cloudflare-vitest-pool-workers-7864

@cloudflare/workers-editor-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13001188073/npm-package-cloudflare-workers-editor-shared-7864

@cloudflare/workers-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13001188073/npm-package-cloudflare-workers-shared-7864

@cloudflare/workflows-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13001188073/npm-package-cloudflare-workflows-shared-7864

Note that these links will no longer work once the GitHub Actions artifact expires.

[email protected] includes the following runtime dependencies:

Package	Constraint	Resolved
`miniflare`	workspace:*	3.20250124.0
`workerd`	1.20250124.0	1.20250124.0
`workerd --version`	1.20250124.0	2025-01-24

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

packages/vite-plugin-cloudflare/README.md

packages/vite-plugin-cloudflare/src/index.ts

jamesopstad

Looks good! Just a few code comments and some changes to the readme.

packages/vite-plugin-cloudflare/src/plugin-config.ts

packages/vite-plugin-cloudflare/src/index.ts

packages/vite-plugin-cloudflare/playground/dev-vars/tsconfig.node.json

packages/vite-plugin-cloudflare/playground/dev-vars/src/index.ts

packages/vite-plugin-cloudflare/playground/dev-vars/.gitignore

packages/vite-plugin-cloudflare/README.md

.changeset/orange-camels-hunt_wrangler.md

.changeset/orange-camels-hunt_vite-plugin.md

.changeset/orange-camels-hunt_wrangler.md

packages/vite-plugin-cloudflare/README.md

vicb · 2025-01-24T19:32:45Z

packages/vite-plugin-cloudflare/playground/dev-vars/.dev.vars.staging

+MY_DEV_VAR_A = "my .dev.vars staging variable A"
+MY_DEV_VAR_B = "my .dev.vars staging variable B"


Suggested change

MY_DEV_VAR_A = "my .dev.vars staging variable A"

MY_DEV_VAR_B = "my .dev.vars staging variable B"

MY_DEV_VAR_A = "my .dev.vars.staging variable A"

MY_DEV_VAR_B = "my .dev.vars.staging variable B"

I prefer not to have the . so that it is clearer that this this is a manually inputted string (otherwise without seeing the implementation someone could thing that this was generated by something like my ${devVarsFileName} variable A)

if you find it confusing I can change the strings to something like MY_DEV_VAR_A = "my .dev.vars variable A for the staging environment (or something like that)

vicb · 2025-01-24T19:39:01Z

packages/vite-plugin-cloudflare/playground/dev-vars/package.json

+		"@cloudflare/vite-plugin": "workspace:*",
+		"@cloudflare/workers-tsconfig": "workspace:*",
+		"@cloudflare/workers-types": "^4.20241230.0",
+		"typescript": "catalog:vite-plugin",


Note, mostly unrelated to this PR:

The vite-plugin catalog pins @types/node but that will not work as expected because of

"overrides": { // ... "@types/node": "$@types/node" },

So there would need to be a more specific override there for the plugin package

packages/vite-plugin-cloudflare/src/index.ts

vicb · 2025-01-24T19:47:01Z

packages/vite-plugin-cloudflare/src/index.ts

 				this.emitFile({
 					type: "asset",
 					fileName: ".assetsignore",
-					source: "wrangler.json",
+					source: `${filesToAssetsIgnore.join("\n")}\n`,


is there any benefit in having a variable here (i.e. do we expect to add more files)
Otherwise it looks looks inlining would be less convoluted

the benefit of the variable are:

it is nicer to have if we can indeed add more files in the future

if is a self documenting variable, the variable name helps (at least to me) understanding that it does

those are not huge benefits I don't mind inlining the array if that's what you strongly prefer

I admit I am not convinced this needs a variable. But I don't care strongly.

me neither 😅, as I said I prefer having the variable but it's not a huge deal either way 🙂

packages/vite-plugin-cloudflare/src/plugin-config.ts

vicb

Looks good - with my limited understanding of the plugin.

But I always find things to comment ;)

petebacondarwin · 2025-01-27T14:54:09Z

packages/vite-plugin-cloudflare/src/index.ts

 				this.emitFile({
 					type: "asset",
 					fileName: ".assetsignore",
-					source: "wrangler.json",
+					source: `${filesToAssetsIgnore.join("\n")}\n`,


I admit I am not convinced this needs a variable. But I don't care strongly.

packages/vite-plugin-cloudflare/src/plugin-config.ts

packages/vite-plugin-cloudflare/src/index.ts

the changes here add tests for making sure that the vite plugin correctly reads secrets from `.dev.vars` files with and without a specified environment as part of this wragler's `unstable_getMiniflareWorkerOptions` utility needed to also be updated to accept the environment name as its second parameter

Update md files Co-authored-by: James Opstad <[email protected]>

Co-authored-by: James Opstad <[email protected]>

Co-authored-by: Victor Berchet <[email protected]>

Co-authored-by: Pete Bacon Darwin <[email protected]>

dario-piotrowicz force-pushed the dario/7850/dev-vars-in-vite-plugin branch from e3e9d3a to 69affde Compare January 22, 2025 15:01

dario-piotrowicz added vite-plugin Relating to the `@cloudflare/vite-plugin` package e2e Run e2e tests on a PR labels Jan 22, 2025

dario-piotrowicz force-pushed the dario/7850/dev-vars-in-vite-plugin branch 3 times, most recently from b1407a5 to 6e539ad Compare January 22, 2025 17:21

dario-piotrowicz commented Jan 22, 2025

View reviewed changes

packages/vite-plugin-cloudflare/README.md Outdated Show resolved Hide resolved

dario-piotrowicz requested review from petebacondarwin and jamesopstad January 22, 2025 17:24

dario-piotrowicz marked this pull request as ready for review January 22, 2025 17:24

dario-piotrowicz requested review from a team as code owners January 22, 2025 17:24

vicb reviewed Jan 22, 2025

View reviewed changes

packages/vite-plugin-cloudflare/README.md Outdated Show resolved Hide resolved

dario-piotrowicz requested a review from vicb January 22, 2025 18:40

dario-piotrowicz marked this pull request as draft January 23, 2025 11:32

dario-piotrowicz commented Jan 23, 2025

View reviewed changes

packages/vite-plugin-cloudflare/src/index.ts Show resolved Hide resolved

dario-piotrowicz force-pushed the dario/7850/dev-vars-in-vite-plugin branch from dbe3e1b to 8689cad Compare January 23, 2025 14:07

dario-piotrowicz marked this pull request as ready for review January 23, 2025 14:22

dario-piotrowicz force-pushed the dario/7850/dev-vars-in-vite-plugin branch from 8689cad to eabe360 Compare January 23, 2025 19:12

jamesopstad reviewed Jan 24, 2025

View reviewed changes

dario-piotrowicz force-pushed the dario/7850/dev-vars-in-vite-plugin branch from 3bb0dd1 to 5cc21f3 Compare January 24, 2025 18:06

dario-piotrowicz requested a review from jamesopstad January 24, 2025 18:12

vicb reviewed Jan 24, 2025

View reviewed changes

.changeset/orange-camels-hunt_wrangler.md Outdated Show resolved Hide resolved

vicb reviewed Jan 24, 2025

View reviewed changes

packages/vite-plugin-cloudflare/README.md Outdated Show resolved Hide resolved

vicb reviewed Jan 24, 2025

View reviewed changes

packages/vite-plugin-cloudflare/README.md Outdated Show resolved Hide resolved

vicb reviewed Jan 24, 2025

View reviewed changes

packages/vite-plugin-cloudflare/src/index.ts Outdated Show resolved Hide resolved

vicb reviewed Jan 24, 2025

View reviewed changes

packages/vite-plugin-cloudflare/src/plugin-config.ts Outdated Show resolved Hide resolved

vicb approved these changes Jan 24, 2025

View reviewed changes

jamesopstad approved these changes Jan 27, 2025

View reviewed changes

dario-piotrowicz force-pushed the dario/7850/dev-vars-in-vite-plugin branch from 23c0acd to b146539 Compare January 27, 2025 11:04

petebacondarwin approved these changes Jan 27, 2025

View reviewed changes

dario-piotrowicz and others added 12 commits January 28, 2025 00:37

Apply suggestions from code review

9a4a696

Update md files Co-authored-by: James Opstad <[email protected]>

update md files

1d7abdf

Apply suggestions from code review

e1b0b5c

Co-authored-by: James Opstad <[email protected]>

remove invalid prepreview script (only used for hacking/debugging)

1f4154f

amend code after having applied suggested changes

476ab01

emit .dev.vars file inside generateBundle

bce010f

Apply suggestions from code review

e081a9a

Co-authored-by: Victor Berchet <[email protected]>

update lock file

1331fee

format files

7d7952d

Update packages/vite-plugin-cloudflare/src/plugin-config.ts

33e3b40

Co-authored-by: Pete Bacon Darwin <[email protected]>

add comment to loadDotEnv

a39093d

dario-piotrowicz force-pushed the dario/7850/dev-vars-in-vite-plugin branch from 744f80b to a39093d Compare January 28, 2025 00:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve and test vite-plugin-cloudflare `.dev.vars` files support #7864

improve and test vite-plugin-cloudflare `.dev.vars` files support #7864

dario-piotrowicz commented Jan 22, 2025 •

edited

Loading

changeset-bot bot commented Jan 22, 2025 •

edited

Loading

github-actions bot commented Jan 22, 2025 •

edited

Loading

jamesopstad left a comment

vicb Jan 24, 2025

dario-piotrowicz Jan 24, 2025

vicb Jan 24, 2025

vicb Jan 24, 2025

dario-piotrowicz Jan 24, 2025

petebacondarwin Jan 27, 2025

dario-piotrowicz Jan 27, 2025

vicb left a comment

petebacondarwin Jan 27, 2025

		MY_DEV_VAR_A = "my .dev.vars staging variable A"
		MY_DEV_VAR_B = "my .dev.vars staging variable B"

improve and test vite-plugin-cloudflare .dev.vars files support #7864

Are you sure you want to change the base?

improve and test vite-plugin-cloudflare .dev.vars files support #7864

Conversation

dario-piotrowicz commented Jan 22, 2025 • edited Loading

changeset-bot bot commented Jan 22, 2025 • edited Loading

🦋 Changeset detected

github-actions bot commented Jan 22, 2025 • edited Loading

jamesopstad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vicb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

improve and test vite-plugin-cloudflare `.dev.vars` files support #7864

improve and test vite-plugin-cloudflare `.dev.vars` files support #7864

dario-piotrowicz commented Jan 22, 2025 •

edited

Loading

changeset-bot bot commented Jan 22, 2025 •

edited

Loading

github-actions bot commented Jan 22, 2025 •

edited

Loading