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

Add mdx filter #550

Merged
merged 7 commits into from Jan 12, 2024
Merged

Add mdx filter #550

merged 7 commits into from Jan 12, 2024

Conversation

kwaa
Copy link
Member

@kwaa kwaa commented Jan 9, 2024

Description

This PR adds an mdx Filter to the MDX plugin.

Check List

  • Have you read the
    CODE OF CONDUCT
  • Have you read the document
    CONTRIBUTING
    • One pull request per feature. If you want to do more than one thing,
      send multiple pull request.
    • Write tests.
    • Run deno fmt to fix the code format before commit.
    • Document any change in the CHANGELOG.md.

@kwaa kwaa requested a review from oscarotero January 9, 2024 07:51
@oscarotero
Copy link
Member

Sorry for the late response. It looks great!
Can you include a test example?

You can take the vto test as example: https://github.com/lumeland/lume/blob/main/tests/assets/vento/vto-filter.page.js

@kwaa
Copy link
Member Author

kwaa commented Jan 12, 2024

Can you include a test example?

I added an example, but I'm not quite sure how filename works though.

lume/plugins/mdx.ts

Lines 150 to 154 in a5430a4

const filter = async (
content: string,
data?: Record<string, unknown>,
): Promise<string> =>
(await engine.render(content, data, "filter.mdx")).toString().trim();

@oscarotero
Copy link
Member

@kwaa In this case it's undefined because it's not a real file.

@kwaa
Copy link
Member Author

kwaa commented Jan 12, 2024

In this case it's undefined because it's not a real file.

If use undefined, it will report error:

Build a mdx site => ./tests/mdx.test.ts:5:6
error: Error: Error rendering the page: /mdx-filter.page.ts
            throw new Error(`Error rendering the page: ${page.sourcePath}`, {
                  ^
    ...
Caused by: TypeError: Path must be a string. Received undefined
    ...

@oscarotero
Copy link
Member

Hmm, okay. It must be here: https://github.com/lumeland/lume/blob/main/plugins/mdx.ts#L73
Not sure if baseUrl is mandatory for mdx. If it is, maybe it can changed to:

const baseUrl = toFileUrl(join(this.baseUrl, filename || "")).href;

@kwaa
Copy link
Member Author

kwaa commented Jan 12, 2024

Not sure if baseUrl is mandatory for mdx. If it is, maybe it can changed to:

const baseUrl = toFileUrl(join(this.baseUrl, filename || "")).href;

Changing to this works fine:

- const baseUrl = toFileUrl(join(this.baseUrl, filename || "")).href;
+ const baseUrl = toFileUrl(join(this.baseUrl, filename || "/")).href;

Using an empty string will cause the JSX import to fail.

@oscarotero oscarotero merged commit 077e1f5 into main Jan 12, 2024
3 checks passed
@oscarotero
Copy link
Member

Great. Good work, thanks you!!

@oscarotero oscarotero deleted the feat/mdx-filter branch January 12, 2024 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants