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

Replacing Existing Node with Next.js Fails to Pass Attributes #63

Closed
arranf opened this issue May 31, 2022 · 7 comments
Closed

Replacing Existing Node with Next.js Fails to Pass Attributes #63

arranf opened this issue May 31, 2022 · 7 comments
Labels
bug Something isn't working

Comments

@arranf
Copy link

arranf commented May 31, 2022

What happened?

Following the docs on nodes and next js nodes is confusing.

Goal: replace the default heading node as the nodes doc demonstrates.

Following the examples in the nodes seems to lead to either the render function being called with no props other than children, or only the transform step being called and not the render step.

Suggestion

Provide more details on the way the nextjs integration interacts with the described transform and render steps, and provide more complete examples for nextjs.

To reproduce

  1. Create a markdoc file with a h1, h2, h3.
  2. Try this basic example (as implied by the combination of nextjs and non-nextjs docs)
import { Title } from '../title';
import { Tag } from '@markdoc/markdoc';


const TitleWrapper = ({ level, id, children }) => {
    console.log('Wrapper called')
    return (
        <Title level={level} id={id}>{children}</Title>
    )
}

export const heading = {
    render: TitleWrapper,
    children: ['inline'],
    attibutes: {
        id: { type: String },
        level: {type: Number, required: true, default: 1}
    }
}
  1. This does render the component however it is never passed any value for level other than the default.

  2. Attempt #2 Try the following

import { Title } from '../title';
import { Tag } from '@markdoc/markdoc';

const TitleWrapper = ({ level, id, children }) => {
    console.log('wrapper')
    const order = Math.min((level || 1) +1, 6) as TitleOrder;
    return (
    
        <Title order={order} id={id}>{children}</Title>
    )
}


function generateID(children, attributes) {
 //... skipped for brevity as not relevant
}

export const heading = {
    render: TitleWrapper,
    children: ['inline'],
    attibutes: {
        id: { type: String },
        level: {type: Number, required: true, default: 1}
    },
    transform(node, config) {
        const attributes = node.transformAttributes(config);
        const children = node.transformChildren(config);

        const id = generateID(children, attributes);

        return new Tag(
        `h${node.attributes['level']}`,
        { ...attributes, id },
        children
        );
  }
}

This does allow for multi-level headings however the render function is never called (no log lines printed or component rendered).

Version

@markdoc/markdoc": 0.1.2, @markdoc/next.js: 0.1.4

Additional context

No response

@arranf arranf added the bug Something isn't working label May 31, 2022
@arranf
Copy link
Author

arranf commented May 31, 2022

It appears the solution is to map to the render function via the component name which appears to need to be in PascalCase for React components and lowercase for native HTML components

I'm not sure of the purpose of the render property on the schema in this case, not sure if this is a bug or just an area of duplication in the API.

I also discovered that the node.transformAttributes call seems to be swallowing the level property - I suspect this is a bug (and perhaps why I do require a transform function at all in this case!) but I don't know enough about Markdoc's API to say for certain what's happening.

I think both the tranformAttributes and the casing requirements should be documented.

Below is a working example.

import { Title, TitleOrder } from '../title';
import { Tag } from '@markdoc/markdoc';


const TitleWrapper = (props) => {
    return (
        <Title order={props.level} id={props.id}>{props.children}</Title>
    )
}


export const heading = {
    render: TitleWrapper,
    children: ['inline'],
    attibutes: {
        id: { type: String },
        level: {type: Number, required: true, default: 1}
    },
    transform(node, config) {
        const children = node.transformChildren(config);
        return new Tag( 
      'Heading',
        { ...node.attributes },
        children
        );
  }
}

@mfix-stripe
Copy link
Contributor

It appears the solution is to map to the render function via the component name which appears to need to be in PascalCase for React components and lowercase for native HTML components

More context on this answer here: #64

If you want to use a custom React component, we recommend creating a Heading component which accepts a level prop, as seen in this discussion. Which then requires returning new Tag('Heading' ...).

However, if you just want to transform your headings (to, for example, always have an ID), then you can use the new Tag(h${node.attributes['level']} ...) pattern.

This is demonstrative of the fact that in React, Heading and h1, h2, etc, are different components. So you have to set the correct component new in your new Tag(...).

@mfix-stripe
Copy link
Contributor

I also discovered that the node.transformAttributes call seems to be swallowing the level property - I suspect this is a bug (and perhaps why I do require a transform function at all in this case!) but I don't know enough about Markdoc's API to say for certain what's happening.

@arranf the level property should not be being swallowed… it is working for us here: https://github.com/markdoc/docs/blob/main/markdoc/nodes/heading.markdoc.js#L25

Can you share a reproduction of this?

@arranf
Copy link
Author

arranf commented May 31, 2022

It appears the solution is to map to the render function via the component name which appears to need to be in PascalCase for React components and lowercase for native HTML components

More context on this answer here: #64

If you want to use a custom React component, we recommend creating a Heading component which accepts a level prop, as seen in this discussion. Which then requires returning new Tag('Heading' ...).

However, if you just want to transform your headings (to, for example, always have an ID), then you can use the new Tag(h${node.attributes['level']} ...) pattern.

This is demonstrative of the fact that in React, Heading and h1, h2, etc, are different components. So you have to set the correct component new in your new Tag(...).

To confirm my understanding can you please answer the following?

  • Assuming I'm using nextjs, for every node will I need a transform function?
  • If that's true, conceptually does the render property just exist to "register" a mapping between the string "Header" and the (in my example) TitleWrapper component?

I think my source of confusion was threefold:

  • The nextjs docs describe nodes being just like tags - where there is no transform function (I got a little mislead there!)
  • Not quite understanding the casing needs of the html vs react components (which are documented, I just didn't realise those steps applied in the Nextjs world!)
  • Not understanding why I need to use Header (which I presume comes from the fact my node is exported as header) in order to render TitleWrapper (this is especially confusing as the transform function is defined on the same object as the render function!)

I also discovered that the node.transformAttributes call seems to be swallowing the level property - I suspect this is a bug (and perhaps why I do require a transform function at all in this case!) but I don't know enough about Markdoc's API to say for certain what's happening.

@arranf the level property should not be being swallowed… it is working for us here: https://github.com/markdoc/docs/blob/main/markdoc/nodes/heading.markdoc.js#L25

Can you share a reproduction of this?

I'll go ahead and create one.

@arranf
Copy link
Author

arranf commented May 31, 2022

I also discovered that the node.transformAttributes call seems to be swallowing the level property - I suspect this is a bug (and perhaps why I do require a transform function at all in this case!) but I don't know enough about Markdoc's API to say for certain what's happening.

@arranf the level property should not be being swallowed… it is working for us here: https://github.com/markdoc/docs/blob/main/markdoc/nodes/heading.markdoc.js#L25

Can you share a reproduction of this?

Repro is here: https://github.com/arranf/markdoc-nextjs-repro

@mfix-stripe
Copy link
Contributor

Repro is here: https://github.com/arranf/markdoc-nextjs-repro

@arranf you just have a typo, attibutes -> attributes 🙂

@arranf
Copy link
Author

arranf commented May 31, 2022

Repro is here: https://github.com/arranf/markdoc-nextjs-repro

@arranf you just have a typo, attibutes -> attributes slightly_smiling_face

🙃 🤡

Well I feel stupid now! Thanks!

@arranf arranf closed this as completed May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants