Skip to content

chore: fix harcoded oauth dev port #1756

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.

Sign up for GitHub

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

Draft
serhalp wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

chore: fix harcoded oauth dev port #1756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions config/env.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import Git from 'simple-git'
import * as process from 'node:process'
import { useNuxt } from 'nuxt/kit'

export { version } from '../package.json'

Expand Down Expand Up @@ -149,13 +150,16 @@ export async function getEnv(isDevelopment: boolean) {
: branch === 'main'
? 'canary'
: 'release'
const nuxt = useNuxt()
const devUrl = isDevelopment ? `http://127.0.0.1:${nuxt.options.devServer.port}` : undefined
Comment on lines +153 to +154
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 28, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, locate and inspect config/env.ts around lines 153-154
find . -name "env.ts" -path "*/config/*" | head -5

Repository: npmx-dev/npmx.dev

Length of output: 76

🏁 Script executed:

# Check the structure around lines 153-154 in config/env.ts
fd "env.ts" -p "config" | head -1 | xargs wc -l

Repository: npmx-dev/npmx.dev

Length of output: 78

🏁 Script executed:

# Get the actual content around lines 153-154
fd "env.ts" -p "config" | head -1 | xargs cat -n | sed -n '145,165p'

Repository: npmx-dev/npmx.dev

Length of output: 729

🏁 Script executed:

# Also check modules/oauth.ts to verify the fallback logic
fd "oauth.ts" -p "modules" | head -1 | xargs cat -n | sed -n '1,30p'

Repository: npmx-dev/npmx.dev

Length of output: 1289

🏁 Script executed:

# Search for the getEnv function definition to understand context
rg "function getEnv|const getEnv" -B 2 -A 5

Repository: npmx-dev/npmx.dev

Length of output: 365

🌐 Web query:

Nuxt 4.3.1 devServer.port type definition and whether it is guaranteed to be defined

💡 Result:

Type definition (Nuxt 4.3.1)

  • In nuxt.config.ts (user config), devServer.port is a number and is optional (you can omit it). The config reference documents it as:
    • Type: number
    • Default: 3000  [1]

Is it guaranteed to be defined?

  • In resolved Nuxt options (nuxt.options), devServer.port is treated as defined (Nuxt applies defaults). The nuxt dev command docs show the fallback chain ending in nuxtOptions.devServer.port (not optional-chained), which implies Nuxt expects it to exist after resolution. [2]

Practical takeaway

  • If you’re reading the user’s raw config object, it may be undefined (because it’s optional in config).
  • If you’re reading nuxt.options.devServer.port inside Nuxt/module hooks after config resolution, you can generally treat it as a number (defaulting to 3000 unless overridden by CLI/env). [1][2]

Sources: [1] Nuxt config reference (devServer.port) [2] nuxt dev command docs (default resolution order)

🏁 Script executed:

# Check if there are any type definitions or nuxt.config in the repo
fd "nuxt.config" | head -5

Repository: npmx-dev/npmx.dev

Length of output: 95

🏁 Script executed:

# Search for devServer port references in the codebase
rg "devServer\.port|devServer\?" -B 2 -A 2

Repository: npmx-dev/npmx.dev

Length of output: 350

🏁 Script executed:

# Check if there's package.json to verify Nuxt version
cat package.json | grep -A 2 -B 2 '"nuxt"'

Repository: npmx-dev/npmx.dev

Length of output: 196

🏁 Script executed:

# Search for all calls to getEnv to verify it's only called in module context
rg "getEnv\(" -B 2 -A 2

Repository: npmx-dev/npmx.dev

Length of output: 1248

🏁 Script executed:

# Also check the full context of config/env.ts to see if there are any type annotations
cat -n config/env.ts | head -20

Repository: npmx-dev/npmx.dev

Length of output: 937

🏁 Script executed:

# Check nuxt.config.ts to see how devServer is configured in this project
cat nuxt.config.ts

Repository: npmx-dev/npmx.dev

Length of output: 10471

Add defensive check for devServer.port to align with strict type-safety guidelines.

Line 154 accesses nuxt.options.devServer.port without validation. Although Nuxt resolves this to a default value of 3000 in module setup hooks, the code should align with the coding guideline requiring strictly type-safe access patterns. A defensive check would improve code robustness:

const nuxt = useNuxt()
const devPort = nuxt.options.devServer?.port
const devUrl = isDevelopment && typeof devPort === 'number' ? `http://127.0.0.1:${devPort}` : undefined

This ensures devUrl is only set when a valid port is present, maintaining consistency with the fallback chain in modules/oauth.ts line 16.

const previewUrl = getPreviewUrl()
const productionUrl = getProductionUrl()
return {
commit,
shortCommit,
branch,
env,
devUrl,
previewUrl,
productionUrl,
prNumber,
Expand Down
4 changes: 2 additions & 2 deletions modules/oauth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ export default defineNuxtModule({
async setup() {
const nuxt = useNuxt()

const { previewUrl, productionUrl } = await getEnv(nuxt.options.dev)
const clientUri = productionUrl || previewUrl || 'http://127.0.0.1:3000'
const { devUrl, previewUrl, productionUrl } = await getEnv(nuxt.options.dev)
const clientUri = productionUrl || previewUrl || devUrl || 'http://127.0.0.1:3000'
Copy link
Member Author

@serhalp serhalp Feb 28, 2026

Choose a reason for hiding this comment

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

We still need this final fallback for the test env.


// bake it into a virtual file
addServerTemplate({
Expand Down
Loading