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

✨ Chore: Extend Markdoc configuration #127

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pietrodev07
Copy link

Description

Copy link

studiocms-deployment-helper bot commented Jun 29, 2024

The preview deployment is ready. 🟢

Open Preview | Open Build Logs

Last updated at: 2024-06-29 10:52:15 CET

Copy link

studiocms-deployment-helper bot commented Jun 29, 2024

The preview deployment is ready. 🟢

Open Preview | Open Build Logs

Last updated at: 2024-06-29 10:51:38 CET

Copy link

studiocms-deployment-helper bot commented Jun 29, 2024

The preview deployment is ready. 🟢

Open Preview | Open Build Logs

Last updated at: 2024-06-29 10:53:44 CET

@Adammatthiesen
Copy link
Member

@pietrodev07
Error from deployment server...
#12 6.177  ERR_PNPM_OUTDATED_LOCKFILE  Cannot install with "frozen-lockfile" because pnpm-lock.yaml is not up to date with packages/studioCMS/package.json

Copy link
Member

@Adammatthiesen Adammatthiesen left a comment

Choose a reason for hiding this comment

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

A good start! I don't have to many issues that need to be addressed right off the bat... Not sure if @jdtjenkins could help with the react parts.... but shouldn't be to hard to figure out!

case 'react':
return Markdoc.renderers.react(content, React);
case 'react-static':
return Markdoc.renderers.reactStatic(content);
Copy link
Member

Choose a reason for hiding this comment

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

What is the output of these? In order for them to work with the StudioCMSRenderer.astro the final output will need to be valid HTML...

Copy link
Author

Choose a reason for hiding this comment

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

react => return ReactNode
react-static => return string so html

Copy link
Member

@Adammatthiesen Adammatthiesen Jun 29, 2024

Choose a reason for hiding this comment

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

for the ReactNode, it might be worth working out a implementation with the Astro ContainerAPI

Copy link
Author

Choose a reason for hiding this comment

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

for the ReactNode, it might be work working out a implementation with the Astro ContainerAPI

yes, but it’s in an experimental phase, so now we can remove the react option and add it later

Copy link
Member

Choose a reason for hiding this comment

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

This whole project(studioCMS) is in experimental state as well (and we are already using the container api 😛 )

Copy link
Author

Choose a reason for hiding this comment

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

This whole project(studioCMS) is in experimental state as well (and we are already using the container api 😛 )

yeah, so we can use container api...

Copy link
Member

Choose a reason for hiding this comment

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

indeed! Hakuna matata!

I like living on the edge.... we are using ALOT of experimental/beta stuff since astroDB is experimental as well

"astro": ">=4.10.3"
"@types/react": "^18.3.3",
"astro": ">=4.10.3",
"react": "^18.3.1"
Copy link
Member

Choose a reason for hiding this comment

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

I don't mind the @types/react being a standard peerDependeny but could we add a peerDependenciesMeta for react to be optional?

example

Copy link
Author

Choose a reason for hiding this comment

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

yeah, make sense, both react and @types/react?

Copy link
Member

Choose a reason for hiding this comment

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

I think just react needs to be moved to an Optional state... but @jdtjenkins could probably give a better answer since i know he had to deal with this when he made the devtoolbar app for AIK

Copy link
Author

Choose a reason for hiding this comment

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

I think just react needs to be moved to an Optional state... but @jdtjenkins could probably give a better answer since i know he had to deal with this when he made the devtoolbar app for AIK

yes, so we wait for his answer

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah for AIK we had react , @types/react and react-dom as optional peer deps if I remember correctly!

{
  "peerDependencies": {
    "react": "^",
    "@types/react": "^",
    "react-dom": "^",
  },
  "peerDependenciesMeta": {
    "react": {
      "optional": true
    },
    "@types/react": {
      "optional": true
    },
    "react-dom": {
      "optional": true
    }
  }
}

https://docs.npmjs.com/cli/v10/configuring-npm/package-json#peerdependenciesmeta

Copy link
Author

Choose a reason for hiding this comment

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

Yeah for AIK we had react , @types/react and react-dom as optional peer deps if I remember correctly!

{
  "peerDependencies": {
    "react": "^",
    "@types/react": "^",
    "react-dom": "^",
  },
  "peerDependenciesMeta": {
    "react": {
      "optional": true
    },
    "@types/react": {
      "optional": true
    },
    "react-dom": {
      "optional": true
    }
  }
}

https://docs.npmjs.com/cli/v10/configuring-npm/package-json#peerdependenciesmeta

@jdtjenkins yeah, thank you!

@Adammatthiesen Adammatthiesen mentioned this pull request Jun 29, 2024
13 tasks
@jdtjenkins
Copy link
Contributor

@pietrodev07 Getting better! I think Adam's right, I think the Container API would be great for this!

Are you able to add a playground project and put together a test page which shows this working too? Then we can easily check that when we make any changes to it, and makes it easier to test it all works!

@Adammatthiesen Adammatthiesen changed the title chore(markdoc): extend markdoc configuration ✨ Chore: Extend Markdoc configuration Jul 1, 2024
@pietrodev07
Copy link
Author

@pietrodev07 Getting better! I think Adam's right, I think the Container API would be great for this!

Are you able to add a playground project and put together a test page which shows this working too? Then we can easily check that when we make any changes to it, and makes it easier to test it all works!

yeah, so I’ll work on it, thanks, man!

@Adammatthiesen
Copy link
Member

yeah, so I’ll work on it, thanks, man!

Hey @pietrodev07 Any updates? We are getting super close to being ready to go to our first beta, and would love for this to be included to provide full functionality for our MarkDoc option!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] Extend current markdoc support
3 participants