Skip to content
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

Deprecate drafts feature #8099

Merged
merged 7 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
6 changes: 6 additions & 0 deletions .changeset/silent-bikes-crash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@astrojs/rss': patch
'astro': patch
---

Deprecate the drafts feature in favour of content collections. If you'd like to create draft pages that's visible in dev but not in prod, you can [migrate to content collections](https://docs.astro.build/en/guides/content-collections/#migrating-from-file-based-routing) and [manually filter out pages](https://docs.astro.build/en/guides/content-collections/#filtering-collection-queries) with the `draft: true` frontmatter property instead.
bluwy marked this conversation as resolved.
Show resolved Hide resolved
10 changes: 8 additions & 2 deletions packages/astro-rss/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ export type RSSOptions = {
stylesheet?: z.infer<typeof rssOptionsValidator>['stylesheet'];
/** Specify custom data in opening of file */
customData?: z.infer<typeof rssOptionsValidator>['customData'];
/** Whether to include drafts or not */
/**
* Whether to include drafts or not
* @deprecated since version 3.0. Use content collections instead.
*/
drafts?: z.infer<typeof rssOptionsValidator>['drafts'];
trailingSlash?: z.infer<typeof rssOptionsValidator>['trailingSlash'];
};
Expand All @@ -45,7 +48,10 @@ export type RSSFeedItem = {
description?: z.infer<typeof rssSchema>['description'];
/** Append some other XML-valid data to this item */
customData?: z.infer<typeof rssSchema>['customData'];
/** Whether draft or not */
/**
* Whether draft or not
* @deprecated since version 3.0. Use content collections instead.
*/
draft?: z.infer<typeof rssSchema>['draft'];
/** Categories or tags related to the item */
categories?: z.infer<typeof rssSchema>['categories'];
Expand Down
5 changes: 5 additions & 0 deletions packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1017,6 +1017,7 @@ export interface AstroUserConfig {
* @name markdown.drafts
* @type {boolean}
* @default `false`
* @deprecated since version 3.0. Use content collections instead.
Copy link
Member

Choose a reason for hiding this comment

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

@delucis - Is there any special formatting we need to adhere to for the deprecated entry here?

Copy link
Member

Choose a reason for hiding this comment

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

We don’t actually display @deprecated comments in docs yet, so now’s our chance to decide a format if we want!

I can imagine generating something like this? Where the contents of the comments follow the leading Deprecated:.

image

Or even something like this depending on how highlighted we want it:

image

So given that, I guess it depends if you think specific wording would be clearest there @sarah11918?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, checking where I adopted the since version X pattern which is from errors-data.ts, it looks like we already render it today:

image

So perhaps another place to align if we want to make @deprecated work nicely everywhere.

Copy link
Member

Choose a reason for hiding this comment

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

Ah OK, yes, we could implement a similar pattern in the config reference if we like how the error generation script does this.

In this example I don’t love how basically a full sentence is split across heading and paragraph, so if we like the chunkier aside rendering, I would probably argue we should end up with something that looks more like:

⚠️ Deprecated
Deprecated since version 2.6. Use content collections instead.

In which case, best for that to be reflected in the JSDoc comment too:

Suggested change
* @deprecated since version 3.0. Use content collections instead.
* @deprecated Deprecated since version 2.6. Use content collections instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed a commit to update the @deprecated tag to be a proper sentence instead 👍

* @description
* Control whether Markdown draft pages should be included in the build.
*
Expand Down Expand Up @@ -1460,6 +1461,10 @@ export interface ComponentInstance {
default: AstroComponentFactory;
css?: string[];
prerender?: boolean;
/**
* Only used for logging if deprecated drafts feature is used
*/
frontmatter?: Record<string, any>;
getStaticPaths?: (options: GetStaticPathsOptions) => GetStaticPathsResult;
}

Expand Down
8 changes: 7 additions & 1 deletion packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import { runHookBuildGenerated } from '../../integrations/index.js';
import { isServerLikeOutput } from '../../prerender/utils.js';
import { BEFORE_HYDRATION_SCRIPT_ID, PAGE_SCRIPT_ID } from '../../vite-plugin-scripts/index.js';
import { AstroError, AstroErrorData } from '../errors/index.js';
import { debug, info } from '../logger/core.js';
import { debug, info, warn } from '../logger/core.js';
import { RedirectSinglePageBuiltModule, getRedirectLocationOrThrow } from '../redirects/index.js';
import { isEndpointResult } from '../render/core.js';
import { createEnvironment, createRenderContext, tryRenderRoute } from '../render/index.js';
Expand Down Expand Up @@ -249,6 +249,12 @@ async function generatePage(
const pageModule = await pageModulePromise();
if (shouldSkipDraft(pageModule, opts.settings)) {
info(opts.logging, null, `${magenta('⚠️')} Skipping draft ${pageData.route.component}`);
// TODO: Remove in Astro 4.0
warn(
opts.logging,
'astro',
`The drafts feature is deprecated. You should migrate to content collections instead. See https://docs.astro.build/en/guides/content-collections/#filtering-collection-queries for more information.`
);
return;
}

Expand Down
10 changes: 10 additions & 0 deletions packages/astro/src/core/render/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {
import { renderPage as runtimeRenderPage } from '../../runtime/server/index.js';
import { attachCookiesToResponse } from '../cookies/index.js';
import { callEndpoint, createAPIContext, type EndpointCallResult } from '../endpoint/index.js';
import { warn } from '../logger/core.js';
import { callMiddleware } from '../middleware/callMiddleware.js';
import { redirectRouteGenerate, redirectRouteStatus, routeIsRedirect } from '../redirects/index.js';
import type { RenderContext } from './context.js';
Expand Down Expand Up @@ -58,6 +59,15 @@ export async function renderPage({ mod, renderContext, env, cookies }: RenderPag
locals: renderContext.locals ?? {},
});

// TODO: Remove in Astro 4.0
if (mod.frontmatter && typeof mod.frontmatter === 'object' && 'draft' in mod.frontmatter) {
warn(
env.logging,
'astro',
`The drafts feature is deprecated and used in ${renderContext.route.component}. You should migrate to content collections instead. See https://docs.astro.build/en/guides/content-collections/#filtering-collection-queries for more information.`
);
}

const response = await runtimeRenderPage(
result,
Component,
Expand Down