Skip to content

Commit

Permalink
Markdoc asset bleed, second try (#7185)
Browse files Browse the repository at this point in the history
* Revert "revert: markdoc asset bleed (#7178)"

This reverts commit 57e65d2.

* fix: missing result param on `renderUniqueStylesheet`

* test: bundled styles (fails!)

* fix: use `type: 'external'` for links

* fix: split Astro components from markdoc config

* test: style bleed (it fails...)

* chore: remove unused util

* fix: revert entry change

* Stop traversing the graph when you encounter a propagated asset

* chore: cleanup unused `entry` prop

* refactor: add isPropagatedAssetsMod check

* chore: remove unused import

* chore: changeset

* Normalize path using vite

* Update packages/integrations/markdoc/src/index.ts

Co-authored-by: Ben Holmes <hey@bholmes.dev>

---------

Co-authored-by: Matthew Phillips <matthew@skypack.dev>
Co-authored-by: bholmesdev <bholmesdev@gmail.com>
Co-authored-by: Matthew Phillips <matthew@matthewphillips.info>
  • Loading branch information
4 people committed May 31, 2023
1 parent c4c086e commit 339529f
Show file tree
Hide file tree
Showing 28 changed files with 653 additions and 131 deletions.
7 changes: 7 additions & 0 deletions .changeset/smart-moles-double.md
@@ -0,0 +1,7 @@
---
'astro': patch
'@astrojs/markdoc': patch
'@astrojs/mdx': patch
---

Bring back improved style and script handling across content collection files. This addresses bugs found in a previous release to `@astrojs/markdoc`.
9 changes: 0 additions & 9 deletions examples/with-markdoc/src/content/config.ts

This file was deleted.

6 changes: 6 additions & 0 deletions packages/astro/src/@types/astro.ts
Expand Up @@ -1276,6 +1276,12 @@ export interface ContentEntryType {
}
): rollup.LoadResult | Promise<rollup.LoadResult>;
contentModuleTypes?: string;
/**
* Handle asset propagation for rendered content to avoid bleed.
* Ex. MDX content can import styles and scripts, so `handlePropagation` should be true.
* @default true
*/
handlePropagation?: boolean;
}

type GetContentEntryInfoReturnType = {
Expand Down
9 changes: 8 additions & 1 deletion packages/astro/src/content/consts.ts
@@ -1,11 +1,18 @@
export const PROPAGATED_ASSET_FLAG = 'astroPropagatedAssets';
export const CONTENT_RENDER_FLAG = 'astroRenderContent';
export const CONTENT_FLAG = 'astroContentCollectionEntry';
export const DATA_FLAG = 'astroDataCollectionEntry';
export const CONTENT_FLAGS = [CONTENT_FLAG, DATA_FLAG, PROPAGATED_ASSET_FLAG] as const;

export const VIRTUAL_MODULE_ID = 'astro:content';
export const LINKS_PLACEHOLDER = '@@ASTRO-LINKS@@';
export const STYLES_PLACEHOLDER = '@@ASTRO-STYLES@@';
export const SCRIPTS_PLACEHOLDER = '@@ASTRO-SCRIPTS@@';

export const CONTENT_FLAGS = [
CONTENT_FLAG,
CONTENT_RENDER_FLAG,
DATA_FLAG,
PROPAGATED_ASSET_FLAG,
] as const;

export const CONTENT_TYPES_FILE = 'types.d.ts';
133 changes: 81 additions & 52 deletions packages/astro/src/content/runtime.ts
Expand Up @@ -269,63 +269,80 @@ async function render({

const baseMod = await renderEntryImport();
if (baseMod == null || typeof baseMod !== 'object') throw UnexpectedRenderError;
const { default: defaultMod } = baseMod;

const { collectedStyles, collectedLinks, collectedScripts, getMod } = baseMod;
if (typeof getMod !== 'function') throw UnexpectedRenderError;
const mod = await getMod();
if (mod == null || typeof mod !== 'object') throw UnexpectedRenderError;
if (isPropagatedAssetsModule(defaultMod)) {
const { collectedStyles, collectedLinks, collectedScripts, getMod } = defaultMod;
if (typeof getMod !== 'function') throw UnexpectedRenderError;
const propagationMod = await getMod();
if (propagationMod == null || typeof propagationMod !== 'object') throw UnexpectedRenderError;

const Content = createComponent({
factory(result, baseProps, slots) {
let styles = '',
links = '',
scripts = '';
if (Array.isArray(collectedStyles)) {
styles = collectedStyles
.map((style: any) => {
return renderUniqueStylesheet(result, {
type: 'inline',
content: style,
});
})
.join('');
}
if (Array.isArray(collectedLinks)) {
links = collectedLinks
.map((link: any) => {
return renderUniqueStylesheet(result, {
type: 'external',
src: prependForwardSlash(link),
});
})
.join('');
}
if (Array.isArray(collectedScripts)) {
scripts = collectedScripts.map((script: any) => renderScriptElement(script)).join('');
}
const Content = createComponent({
factory(result, baseProps, slots) {
let styles = '',
links = '',
scripts = '';
if (Array.isArray(collectedStyles)) {
styles = collectedStyles
.map((style: any) => {
return renderUniqueStylesheet(result, {
type: 'inline',
content: style,
});
})
.join('');
}
if (Array.isArray(collectedLinks)) {
links = collectedLinks
.map((link: any) => {
return renderUniqueStylesheet(result, {
type: 'external',
src: prependForwardSlash(link),
});
})
.join('');
}
if (Array.isArray(collectedScripts)) {
scripts = collectedScripts.map((script: any) => renderScriptElement(script)).join('');
}

let props = baseProps;
// Auto-apply MDX components export
if (id.endsWith('.mdx')) {
props = {
components: mod.components ?? {},
...baseProps,
};
}
let props = baseProps;
// Auto-apply MDX components export
if (id.endsWith('mdx')) {
props = {
components: propagationMod.components ?? {},
...baseProps,
};
}

return createHeadAndContent(
unescapeHTML(styles + links + scripts) as any,
renderTemplate`${renderComponent(result, 'Content', mod.Content, props, slots)}`
);
},
propagation: 'self',
});
return createHeadAndContent(
unescapeHTML(styles + links + scripts) as any,
renderTemplate`${renderComponent(
result,
'Content',
propagationMod.Content,
props,
slots
)}`
);
},
propagation: 'self',
});

return {
Content,
headings: mod.getHeadings?.() ?? [],
remarkPluginFrontmatter: mod.frontmatter ?? {},
};
return {
Content,
headings: propagationMod.getHeadings?.() ?? [],
remarkPluginFrontmatter: propagationMod.frontmatter ?? {},
};
} else if (baseMod.Content && typeof baseMod.Content === 'function') {
return {
Content: baseMod.Content,
headings: baseMod.getHeadings?.() ?? [],
remarkPluginFrontmatter: baseMod.frontmatter ?? {},
};
} else {
throw UnexpectedRenderError;
}
}

export function createReference({ lookupMap }: { lookupMap: ContentLookupMap }) {
Expand Down Expand Up @@ -362,3 +379,15 @@ export function createReference({ lookupMap }: { lookupMap: ContentLookupMap })
});
};
}

type PropagatedAssetsModule = {
__astroPropagation: true;
getMod: () => Promise<any>;
collectedStyles: string[];
collectedLinks: string[];
collectedScripts: string[];
};

function isPropagatedAssetsModule(module: any): module is PropagatedAssetsModule {
return typeof module === 'object' && module != null && '__astroPropagation' in module;
}
2 changes: 1 addition & 1 deletion packages/astro/src/content/template/virtual-mod.mjs
Expand Up @@ -46,7 +46,7 @@ function createGlobLookup(glob) {
}

const renderEntryGlob = import.meta.glob('@@RENDER_ENTRY_GLOB_PATH@@', {
query: { astroPropagatedAssets: true },
query: { astroRenderContent: true },
});
const collectionToRenderEntryMap = createCollectionToGlobResultMap({
globResult: renderEntryGlob,
Expand Down
3 changes: 2 additions & 1 deletion packages/astro/src/content/utils.ts
Expand Up @@ -15,6 +15,7 @@ import type {
} from '../@types/astro.js';
import { VALID_INPUT_FORMATS } from '../assets/consts.js';
import { AstroError, AstroErrorData } from '../core/errors/index.js';

import { formatYAMLException, isYAMLException } from '../core/errors/utils.js';
import { CONTENT_FLAGS, CONTENT_TYPES_FILE } from './consts.js';
import { errorMap } from './error-map.js';
Expand Down Expand Up @@ -331,7 +332,7 @@ export function parseFrontmatter(fileContents: string, filePath: string) {
*/
export const globalContentConfigObserver = contentObservable({ status: 'init' });

export function hasContentFlag(viteId: string, flag: (typeof CONTENT_FLAGS)[number]) {
export function hasContentFlag(viteId: string, flag: (typeof CONTENT_FLAGS)[number]): boolean {
const flags = new URLSearchParams(viteId.split('?')[1] ?? '');
return flags.has(flag);
}
Expand Down
43 changes: 31 additions & 12 deletions packages/astro/src/content/vite-plugin-content-assets.ts
@@ -1,4 +1,5 @@
import { pathToFileURL } from 'url';
import { extname } from 'node:path';
import { pathToFileURL } from 'node:url';
import type { Plugin } from 'vite';
import type { AstroSettings } from '../@types/astro.js';
import { moduleIsTopLevelPage, walkParentInfos } from '../core/build/graph.js';
Expand All @@ -11,16 +12,13 @@ import { joinPaths, prependForwardSlash } from '../core/path.js';
import { getStylesForURL } from '../core/render/dev/css.js';
import { getScriptsForURL } from '../core/render/dev/scripts.js';
import {
CONTENT_RENDER_FLAG,
LINKS_PLACEHOLDER,
PROPAGATED_ASSET_FLAG,
SCRIPTS_PLACEHOLDER,
STYLES_PLACEHOLDER,
} from './consts.js';

function isPropagatedAsset(viteId: string) {
const flags = new URLSearchParams(viteId.split('?')[1]);
return flags.has(PROPAGATED_ASSET_FLAG);
}
import { hasContentFlag } from './utils.js';

export function astroContentAssetPropagationPlugin({
mode,
Expand All @@ -32,13 +30,31 @@ export function astroContentAssetPropagationPlugin({
let devModuleLoader: ModuleLoader;
return {
name: 'astro:content-asset-propagation',
enforce: 'pre',
async resolveId(id, importer, opts) {
if (hasContentFlag(id, CONTENT_RENDER_FLAG)) {
const base = id.split('?')[0];

for (const { extensions, handlePropagation = true } of settings.contentEntryTypes) {
if (handlePropagation && extensions.includes(extname(base))) {
return this.resolve(`${base}?${PROPAGATED_ASSET_FLAG}`, importer, {
skipSelf: true,
...opts,
});
}
}
// Resolve to the base id (no content flags)
// if Astro doesn't need to handle propagation.
return this.resolve(base, importer, { skipSelf: true, ...opts });
}
},
configureServer(server) {
if (mode === 'dev') {
devModuleLoader = createViteLoader(server);
}
},
async transform(_, id, options) {
if (isPropagatedAsset(id)) {
if (hasContentFlag(id, PROPAGATED_ASSET_FLAG)) {
const basePath = id.split('?')[0];
let stringifiedLinks: string, stringifiedStyles: string, stringifiedScripts: string;

Expand Down Expand Up @@ -73,14 +89,17 @@ export function astroContentAssetPropagationPlugin({
}

const code = `
export async function getMod() {
async function getMod() {
return import(${JSON.stringify(basePath)});
}
export const collectedLinks = ${stringifiedLinks};
export const collectedStyles = ${stringifiedStyles};
export const collectedScripts = ${stringifiedScripts};
const collectedLinks = ${stringifiedLinks};
const collectedStyles = ${stringifiedStyles};
const collectedScripts = ${stringifiedScripts};
const defaultMod = { __astroPropagation: true, getMod, collectedLinks, collectedStyles, collectedScripts };
export default defaultMod;
`;

// ^ Use a default export for tools like Markdoc
// to catch the `__astroPropagation` identifier
return { code, map: { mappings: '' } };
}
},
Expand Down
28 changes: 16 additions & 12 deletions packages/astro/src/core/render/dev/vite.ts
@@ -1,7 +1,6 @@
import type { ModuleLoader, ModuleNode } from '../../module-loader/index';

import npath from 'path';
import { PROPAGATED_ASSET_FLAG } from '../../../content/consts.js';
import { SUPPORTED_MARKDOWN_FILE_EXTENSIONS } from '../../constants.js';
import { unwrapId } from '../../util.js';
import { isCSSRequest } from './util.js';
Expand All @@ -10,9 +9,10 @@ import { isCSSRequest } from './util.js';
* List of file extensions signalling we can (and should) SSR ahead-of-time
* See usage below
*/
const fileExtensionsToSSR = new Set(['.astro', ...SUPPORTED_MARKDOWN_FILE_EXTENSIONS]);
const fileExtensionsToSSR = new Set(['.astro', '.mdoc', ...SUPPORTED_MARKDOWN_FILE_EXTENSIONS]);

const STRIP_QUERY_PARAMS_REGEX = /\?.*$/;
const ASTRO_PROPAGATED_ASSET_REGEX = /\?astroPropagatedAssets/;

/** recursively crawl the module graph to get all style files imported by parent id */
export async function* crawlGraph(
Expand All @@ -23,7 +23,6 @@ export async function* crawlGraph(
): AsyncGenerator<ModuleNode, void, unknown> {
const id = unwrapId(_id);
const importedModules = new Set<ModuleNode>();
if (new URL(id, 'file://').searchParams.has(PROPAGATED_ASSET_FLAG)) return;

const moduleEntriesForId = isRootFile
? // "getModulesByFile" pulls from a delayed module cache (fun implementation detail),
Expand All @@ -44,7 +43,11 @@ export async function* crawlGraph(
if (id === entry.id) {
scanned.add(id);
const entryIsStyle = isCSSRequest(id);

for (const importedModule of entry.importedModules) {
// A propagation stopping point is a module with the ?astroPropagatedAssets flag.
// When we encounter one of these modules we don't want to continue traversing.
let isPropagationStoppingPoint = false;
// some dynamically imported modules are *not* server rendered in time
// to only SSR modules that we can safely transform, we check against
// a list of file extensions based on our built-in vite plugins
Expand All @@ -60,15 +63,14 @@ export async function* crawlGraph(
if (entryIsStyle && !isCSSRequest(importedModulePathname)) {
continue;
}
const isFileTypeNeedingSSR = fileExtensionsToSSR.has(
npath.extname(importedModulePathname)
);
isPropagationStoppingPoint = ASTRO_PROPAGATED_ASSET_REGEX.test(importedModule.id);
if (
fileExtensionsToSSR.has(
npath.extname(
// Use `id` instead of `pathname` to preserve query params.
// Should not SSR a module with an unexpected query param,
// like "?astroPropagatedAssets"
importedModule.id
)
)
isFileTypeNeedingSSR &&
// Should not SSR a module with ?astroPropagatedAssets
!isPropagationStoppingPoint
) {
const mod = loader.getModuleById(importedModule.id);
if (!mod?.ssrModule) {
Expand All @@ -80,7 +82,9 @@ export async function* crawlGraph(
}
}
}
importedModules.add(importedModule);
if(!isPropagationStoppingPoint) {
importedModules.add(importedModule);
}
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion packages/astro/src/vite-plugin-head/index.ts
Expand Up @@ -9,7 +9,8 @@ import { getTopLevelPages, walkParentInfos } from '../core/build/graph.js';
import type { BuildInternals } from '../core/build/internal.js';
import { getAstroMetadata } from '../vite-plugin-astro/index.js';

const injectExp = /^\/\/\s*astro-head-inject/;
// Detect this in comments, both in .astro components and in js/ts files.
const injectExp = /(^\/\/|\/\/!)\s*astro-head-inject/;

export default function configHeadVitePlugin({
settings,
Expand All @@ -32,6 +33,7 @@ export default function configHeadVitePlugin({
seen.add(id);
const mod = server.moduleGraph.getModuleById(id);
const info = this.getModuleInfo(id);

if (info?.meta.astro) {
const astroMetadata = getAstroMetadata(info);
if (astroMetadata) {
Expand Down

0 comments on commit 339529f

Please sign in to comment.