Avoid importing vite in db utils #10946
Conversation
🦋 Changeset detectedLatest commit: 477c330 The changes in this PR will be included in the next version bump. 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 |
| @@ -1,8 +1,12 @@ | |||
| export { defineDbIntegration } from './core/utils.js'; | |||
| import type { AstroIntegration } from 'astro'; | |||
| import { tableSchema } from './core/schemas.js'; | |||
There was a problem hiding this comment.
This is still importing from core/schemas.js which imports core/utils.js which imports vite.
The rule is that runtime should not import inside in core. I don't know if this problem is solvable in this way.
There was a problem hiding this comment.
core/schemas no longer imports Vite — I removed/refactored that.
There was a problem hiding this comment.
Yeah, I see that now, but this could easily be broken by a new PR that does need something else from utils. We try to stick with the rule that runtime code only imports runtime code and not core, to prevent mistakes.
| @@ -4,7 +4,6 @@ import { type ZodTypeDef, z } from 'zod'; | |||
| import { SERIALIZED_SQL_KEY, type SerializedSQL } from '../runtime/types.js'; | |||
| import { errorMap } from './integration/error-map.js'; | |||
| import type { NumberColumn, TextColumn } from './types.js'; | |||
| import { mapObject } from './utils.js'; | |||
There was a problem hiding this comment.
core/utils imports Vite so we want to avoid importing it — moved mapObject inline into this file as it is only used here
| export function defineDbIntegration(integration: AstroDbIntegration): AstroIntegration { | ||
| return integration; | ||
| } |
There was a problem hiding this comment.
Moved this straight to the user-facing utils library to avoid importing it from here where Vite is also imported
Changes
@astrojs/db/utilsTesting
Docs
n/a — bug fix