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

fix: image migration issues #965

Merged
merged 5 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
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
28 changes: 26 additions & 2 deletions __tests__/compilers/compatability.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ describe('compatability with RDMD', () => {
};

expect(mdx(ast).trim()).toMatchInlineSnapshot(`
"<Image align="center" width="300px" src="https://drastik.ch/wp-content/uploads/2023/06/blackcat.gif" alt="" title="">
"<Image align="center" width="300px" src="https://drastik.ch/wp-content/uploads/2023/06/blackcat.gif">
hello **cat**
</Image>"
`);
Expand Down Expand Up @@ -205,7 +205,7 @@ This is an image: <img src="http://example.com/#\\>" >
});

describe('<HTMLBlock> wrapping', () => {
// configure({ defaultIgnore: undefined });
// configure({ defaultIgnore: undefined });

const rawStyle = `<style data-testid="style-tag">
p {
Expand Down Expand Up @@ -267,4 +267,28 @@ This is an image: <img src="http://example.com/#\\>" >
expect(spy).toHaveBeenCalledTimes(0);
});
});

it('can parse and transform magic image block AST to MDX', () => {
const md = `
[block:image]
{
"images": [
{
"image": [
"https://files.readme.io/4a1c7a0-Iphone.jpeg",
null,
""
],
"align": "center",
"sizing": "250px"
}
]
}
[/block]
`;

const rmdx = mdx(rdmd.mdast(md));

expect(rmdx).toMatch('<Image align="center" width="250px" src="https://files.readme.io/4a1c7a0-Iphone.jpeg" />');
});
});
12 changes: 11 additions & 1 deletion __tests__/compilers/images.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,20 @@ describe('image compiler', () => {
expect(mdx(mdast(doc))).toMatch(doc);
});

it('ignores empty (undefined, null, or "") attributes', () => {
const doc = '<Image src="/path/to/image.png" border={true} alt="" title={null} align={undefined} />';

expect(mdx(mdast(doc))).toMatch('<Image border={true} src="/path/to/image.png" />');
});

it('correctly serializes an Image component with expression attributes back to MDX', () => {
const doc = '<Image src="/path/to/image.png" border={false} />';

expect(mdx(mdast(doc))).toMatch(doc);
expect(mdx(mdast(doc))).toMatch('![](/path/to/image.png)');

const doc2 = '<Image src="/path/to/image.png" border={true} />';

expect(mdx(mdast(doc2))).toMatch('<Image border={true} src="/path/to/image.png" />');
});

it('correctly serializes an Image component with an undefined expression attributes back to MDX', () => {
Expand Down
2 changes: 1 addition & 1 deletion __tests__/fixtures/image-block-no-attrs.mdx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Lorem ipsum dolor sit amet consectetur adipisicing elit. Possimus magni delectus dolorum velit sit laboriosam a quos? Animi ut quam impedit inventore ratione. Delectus quaerat similique natus optio, magnam quam deleniti, iste fugiat ipsam sapiente modi hic cupiditate sint. Quod dolor, inventore facere laudantium facilis molestias reprehenderit. Consectetur, inventore doloremque.

<Image align="left" className="" width="50% " src="https://files.readme.io/327e65d-image.png" />
<Image align="left" width="50% " src="https://files.readme.io/327e65d-image.png" />

# Hello world

Expand Down
3 changes: 2 additions & 1 deletion __tests__/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const silenceConsole =
};

export const execute = async (doc: string, compileOpts = {}, runOpts = {}) => {
const module = await run(compile(doc, compileOpts), runOpts);
const code = compile(doc, compileOpts);
const module = await run(code, runOpts);
return module.default;
};
1 change: 1 addition & 0 deletions __tests__/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ test('image', async () => {
const md = '![Image](http://example.com/image.png)';
const component = await execute(md);
const { container } = render(React.createElement(component));

expect(container.innerHTML).toMatchSnapshot();
});

Expand Down
38 changes: 15 additions & 23 deletions __tests__/transformers/images.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('images transformer', () => {
const tree = mdast(md);

expect(tree.children[0].type).toBe('image-block');
expect(tree.children[0].data.hProperties.src).toBe('https://example.com/image.jpg');
expect(tree.children[0].src).toBe('https://example.com/image.jpg');
});

it('can parse the caption markdown to children', () => {
Expand All @@ -23,29 +23,21 @@ describe('images transformer', () => {
expect(tree.children[0].children[0].children[2].type).toBe('emphasis');
});

it('can parse and transform magic image block AST to MDX', () => {
const rdmd = `
[block:image]
{
"images": [
{
"image": [
"https://files.readme.io/4a1c7a0-Iphone.jpeg",
null,
""
],
"align": "center",
"sizing": "250px"
}
]
}
[/block]
it('can parse attributes', () => {
const md = `
<Image
align="left"
alt="Some helpful text"
src="https://example.com/image.jpg"
title="Testing"
width="100px"
/>
`;
const tree = mdast(md);

const rmdx = mdx(mdastLegacy(rdmd));

expect(rmdx).toMatch(
'<Image align="center" className="" width="250px" src="https://files.readme.io/4a1c7a0-Iphone.jpeg" />',
);
expect(tree.children[0].align).toBe('left');
expect(tree.children[0].alt).toBe('Some helpful text');
expect(tree.children[0].title).toBe('Testing');
expect(tree.children[0].width).toBe('100px');
});
});
25 changes: 14 additions & 11 deletions processor/transform/images.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,30 +14,34 @@ const imageTransformer = () => (tree: Node) => {

const [{ alt, url, title }] = node.children as any;

const newNode = {
type: NodeTypes.imageBlock,
const attrs = {
alt,
title,
url,
children: [],
src: url,
};

const newNode: ImageBlock = {
type: NodeTypes.imageBlock,
...attrs,
/*
* @note: Using data.hName here means that we don't have to transform
* this to an MdxJsxFlowElement, and rehype will transform it correctly
*/
data: {
hName: 'img',
hProperties: {
...(alt && { alt }),
src: url,
...(title && { title }),
},
hProperties: attrs,
},
position: node.position,
} as ImageBlock;
};

parent.children.splice(i, 1, newNode);
});

const isImage = (node: MdxJsxFlowElement) => node.name === 'Image';

visit(tree, isImage, (node: MdxJsxFlowElement) => {
const attrs = getAttrs<ImageBlock['data']['hProperties']>(node);
const attrs = getAttrs<ImageBlock>(node);

if (attrs.caption) {
node.children = mdast(attrs.caption).children;
Expand All @@ -48,4 +52,3 @@ const imageTransformer = () => (tree: Node) => {
};

export default imageTransformer;

26 changes: 20 additions & 6 deletions processor/transform/readme-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,30 @@ const coerceJsxToMd =
parent.children[index] = mdNode;
} else if (node.name === 'Image') {
const { position } = node;
const { alt = '', url, title = null } = getAttrs<Pick<ImageBlock, 'alt' | 'title' | 'url'>>(node);
const attrs = getAttrs<ImageBlock['data']['hProperties']>(node);
const {
alt = '',
align,
border,
caption,
title = null,
width,
src,
} = getAttrs<Pick<ImageBlock, 'alt' | 'align' | 'border' | 'title' | 'width' | 'src' | 'caption'>>(node);
rafegoldberg marked this conversation as resolved.
Show resolved Hide resolved

const attrs = {
...(align && { align }),
...(border && { border }),
...(src && { src }),
...(width && { width }),
alt,
children: caption ? mdast(caption).children : (node.children as any),
title,
};

const mdNode: ImageBlock = {
alt,
...attrs,
position,
children: attrs.caption ? mdast(attrs.caption).children : (node.children as any),
title,
type: NodeTypes.imageBlock,
url: url || attrs.src,
data: {
hName: 'img',
hProperties: attrs,
Expand Down
6 changes: 3 additions & 3 deletions processor/transform/readme-to-mdx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ const readmeToMdx = (): Transform => tree => {
parent.children.splice(index, 1, {
type: 'image',
children: [],
url: image.url,
title: image.title,
alt: image.alt,
...(image.src && { url: image.src }),
...(image.title && { title: image.title }),
...(image.alt && { alt: image.alt }),
} as Image);
}
});
Expand Down
10 changes: 5 additions & 5 deletions processor/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export const getHPropKeys = <T>(node: Node): any => {
export const getAttrs = <T>(jsx: MdxJsxFlowElement | MdxJsxTextElement): any =>
jsx.attributes.reduce((memo, attr) => {
if ('name' in attr) {
if (typeof attr.value === 'string') {
if (typeof attr.value === 'string' || attr.value === null) {
memo[attr.name] = attr.value;
Copy link
Contributor

Choose a reason for hiding this comment

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

So we intentionally want to set the attribute to null here? May I ask why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wasn't sure what to do with it. But I think it should actual set the value to true

} else if (attr.value.value !== 'undefined') {
memo[attr.name] = JSON.parse(attr.value.value);
Expand Down Expand Up @@ -155,14 +155,14 @@ export const reformatHTML = (html: string, indent: number = 2): string => {
export const toAttributes = (object: Record<string, any>, keys: string[] = []): MdxJsxAttribute[] => {
let attributes: MdxJsxAttribute[] = [];
Object.entries(object).forEach(([name, v]) => {
if ((keys.length > 0 && !keys.includes(name)) || typeof v === 'undefined') return;
if (keys.length > 0 && !keys.includes(name)) return;

let value: MdxJsxAttributeValueExpression | string;

if (typeof v === 'string') {
value = v;
} else if (typeof v === 'undefined') {
if (typeof v === 'undefined' || v === null || v === '') {
return;
} else if (typeof v === 'string') {
value = v;
} else {
/* values can be null, undefined, string, or a expression, eg:
*
Expand Down
27 changes: 13 additions & 14 deletions types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,24 +72,23 @@ interface HTMLBlock extends Node {
};
}

interface ImageBlock extends Parent {
type: NodeTypes.imageBlock;
url: string;
interface ImageBlockAttrs {
align?: string;
alt: string;
border?: string;
caption?: string;
className?: string;
lazy?: boolean;
src: string;
title: string;
width?: string;
}

interface ImageBlock extends ImageBlockAttrs, Parent {
type: NodeTypes.imageBlock;
data: Data & {
hName: 'img';
hProperties: {
align?: string;
alt?: string;
caption?: string;
border?: string;
src: string;
title?: string;
width?: string;
lazy?: boolean;
className?: string;
};
hProperties: ImageBlockAttrs;
};
}

Expand Down
Loading