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
refactor(nuxt): import useNitroApp
from subpath
#22785
Conversation
Run & review this pull request in StackBlitz Codeflow. |
@@ -2,7 +2,8 @@ import { joinURL, withQuery } from 'ufo' | |||
import type { NitroErrorHandler } from 'nitropack' | |||
import type { H3Error } from 'h3' | |||
import { getRequestHeaders, send, setResponseHeader, setResponseStatus } from 'h3' | |||
import { useNitroApp, useRuntimeConfig } from '#internal/nitro' | |||
import { useRuntimeConfig } from '#internal/nitro' | |||
import { useNitroApp } from '#internal/nitro/app' |
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.
Have you tried with import { useNitroApp } from '#imports'
? it should be safer to not introduce internal dependencies.
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.
This isn't introducing a dependency - we already import useNitroApp
elsewhere from this path.
Using #imports
in the Nuxt source code means we don't get type support. (This is soluble but requires a bit of work.)
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.
I would appreciate if we can try to avoid internal imports also for other places sooner ππΌ (We can internally add type back I guess, from one place)
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.
I'm a bit confused. I removed this very import but you then said we didn't need to remove it, here:
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.
Anyway, I agree supporting #imports
would be nice internally. π
I'll go ahead and merge this PR as it doesn't change internal situation and we can look to move safely to #imports
at any point after that.
Hi guys, I still get this error in 3.7.0 with a fresh installation #18789 (comment) server/api/test.js
log
|
Reopened to investigate. |
So for now we can fix it with manually importing it.
|
π Linked issue
#18789 (comment)
β Type of change
π Description
This ensures we import
useNitroApp
from correct path to avoid chunk issue with rollup.π Checklist