fix: pass auth profiles to vite#14588
Conversation
🦋 Changeset detectedLatest commit: 7400be0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
Summary: adds an import-time initProfileResolution() in the public API barrel that resolves the auth profile from directory bindings against process.cwd() and calls setProfile, so the Vite plugin picks up directory-activated profiles.
Ranked issues:
-
(High) Profile is resolved against
process.cwd(), but the CLI resolves against the config file's directory. This means the fix silently does nothing when Vite is run from a directory other than the one containingwrangler.jsonc(monorepo root, subdir, custom--config). The directory binding created bywrangler auth activateis keyed on the config/project dir, so aprocess.cwd()lookup will miss it and fall back todefault— reproducing the original bug. See detail on the inline comment. -
(Medium) Import-time global side effect for all consumers. This runs for every importer of
wrangler/wrangler/api(including theunstable_devtest harness), doing a filesystem read ofdirectory-bindings.jsonand mutating the global OAuth singleton viasetProfilemerely as a side effect ofimport. That's a behavior change beyond the Vite plugin; worth confirming it doesn't surpriseunstable_devusers or interfere with tests. -
(Medium) The doc comment says "the CLI's yargs handler will override this later with the explicit
--profileflag". That's true for the CLI, but the Vite plugin — the actual consumer being fixed — has no yargs handler and no way to pass--profile, so directory-binding resolution atprocess.cwd()is the only path. The comment is misleading about how Vite obtains an explicit profile. -
(Low) No changeset and no tests for a user-facing behavior change to a published package. changeset-bot already flagged the missing changeset; per repo conventions this needs a
patchchangeset forwrangler(and ideally a test asserting the Vite/library path respects an activated profile).
| * override this later with the explicit `--profile` flag if one was provided. | ||
| */ | ||
| export function initProfileResolution(): void { | ||
| const profile = createWranglerProfileStore().resolve({ |
There was a problem hiding this comment.
resolve uses process.cwd(), but the CLI handler resolves the profile against the config file's directory (register-yargs-command.ts:154-160):
const cwd = firstConfigPath ? path.dirname(path.resolve(firstConfigPath)) : process.cwd();
const profile = createWranglerProfileStore().resolve({ profile: args.profile, cwd });Directory bindings from wrangler auth activate are keyed on the project/config directory. If Vite is launched from anywhere other than that directory (monorepo root, a subdir, or with a configPath pointing elsewhere), this lookup misses the binding and falls back to "default" — i.e. the original bug (#14580) still reproduces in those layouts.
Because this runs at import time it can't know the eventual configPath. Consider resolving lazily from the Vite plugin once configPath is known (the plugin already has it in plugin-config.ts) rather than a blanket process.cwd() resolution here — e.g. export a setter/resolver the plugin calls with path.dirname(configPath). At minimum, please confirm the intended behavior when cwd !== dirname(configPath).
|
Review posted successfully to PR #14588. I reviewed the PR, which adds an import-time
|
@cloudflare/autoconfig
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
bb98bf0 to
8e9a085
Compare
|
✅ All changesets look good |
8e9a085 to
7400be0
Compare
Fixes #14580
A picture of a cute animal (not mandatory, but encouraged)