refactor: dedicated App for astro server and for external servers
#14345
Conversation
|
github-actions
bot
added
pkg: integration
ematipico
changed the title
ascorbic
left a comment
ascorbic
left a comment
There was a problem hiding this comment.
I am a little unclear why there needs to be two different sources of truth. Can it use the manifest settings in both cases?
packages/integrations/cloudflare/test/fixtures/vite-plugin/src/worker.ts
Outdated
Show resolved
Hide resolved
packages/integrations/cloudflare/test/fixtures/vite-plugin/src/worker.ts
Outdated
Show resolved
Hide resolved
ematipico
changed the title
App for astro server and for external servers
| let info = this.getModuleInfo(result.id); | ||
| const astro = info && getAstroMetadata(info); | ||
| if (astro) { | ||
| if (astro && isRunnableDevEnvironment(environment)) { |
There was a problem hiding this comment.
Does this not work with other envionrments?
There was a problem hiding this comment.
I'm not sure I understand the question, but without this guard, the Cloudflare plugin emits errors
There was a problem hiding this comment.
I meant remove the guard in configureServer instead, so that it populates environment and can run in workerd too
|
|
||
| import type { SharpImageServiceConfig } from '../assets/services/sharp.js'; | ||
|
|
||
| export { createConsoleLogger, createNodeLogger } from '../core/config/index.js'; |
There was a problem hiding this comment.
Is there a reason to export this from here?
There was a problem hiding this comment.
Good catch. It's not needed anymore
Changes
This PR does a further refactor of our internals. The main issue I was facing was the fact that I was trying to use the
DevAppboth in our internal dev server and the dev server spawned by external plugins such as Cloudflare.This wasn't working and driving me nuts because
DevAppuses a lot of Node.js APIs across the board. It was mental. So after thinking more about what @ascorbic said, we should try to useAppinstead, it actually made sense and it was the right approach. However, in dev, we handle a few things differently compared to production. For example, we handle500errors differently in dev.So now, we have two apps:
AstroServerApp(which uses its ownAstroServerPipeline)DevApp(which uses its ownDevPipeline)AstroServerAppIt uses the code and business logic we currently use in our Astro server. The old
DevPipelinehas been renamed toAstroServerPipelineDevAppThis new
DevAppis mostly the same asApp. They both use the same methods (such asrender), they slightly differ in thematchimplementation, for example.Since
AppandDevAppshare the same logic, they are runtime-agnostic.The new
DevPipeline, for now, is a copy-paste ofAppPipeline, however, we changed the logic of the functiongetComponentByRoute. In development, we don't have the compiled Astro files, so we need to dynamically import them e.g.import("path/to/file.astro). Eventually, in dev, our rollup plugin is triggered, and it should return the compiled code.Testing
Minimal and react examples still work locally
Docs