hashicorp / next-mdx-remote

Load MDX content from anywhere
Mozilla Public License 2.0
2.58k stars 139 forks source link

Memory Leak while working with Nextjs 14 #429

Open talpx0 opened 7 months ago

talpx0 commented 7 months ago
function createHeading(level: number) {
    const fileSet:Set<string> = new Set() 
    // eslint-disable-next-line react/display-name
    return ({children}: {children: React.ReactNode }) => {
      let slug = slugify(children!.toString()); 
      let uniquePath = slug;
      let counter = 1;
      while (fileSet.has(uniquePath)) {
          uniquePath = `${slug}-${counter}`;
          counter++;
      }
      fileSet.add(uniquePath);
      return React.createElement(
          `h${level}`,
          { id: uniquePath},
          React.createElement('a', {
            href: `#${uniquePath}`,
            key: `link-${uniquePath}`,
            className: 'anchor mdx-header-anchor',
          },children)
      );
    };
}

let components = {
  h1: createHeading(1),
  h2: createHeading(2),
  h3: createHeading(3),
  h4: createHeading(4),
  h5: createHeading(5),
  h6: createHeading(6),
  Image: RoundedImage,
  a: CustomLink,
  Callout,
  ProsCard,
  ConsCard,
  StaticTweet: TweetComponent,
  Table,
};

export function CustomMDX(props:any) {
  return (
    <MDXRemote
      {...props}
      components={{ ...components, ...(props.components || {}) }}
    />
  );
}

basically I fixed error after move the "const fileSet:Set = new Set()" into return function like this , before , it is a closure and I don't know why it will cause memory leak ? is it because of mdx remote have cache mechanism , and it will cache the state instead of clean it ?

function createHeading(level: number) {
    // eslint-disable-next-line react/display-name
    return ({children}: {children: React.ReactNode }) => {
      const fileSet:Set<string> = new Set() 
      let slug = slugify(children!.toString()); 
      let uniquePath = slug;
      let counter = 1;
      while (fileSet.has(uniquePath)) {
          uniquePath = `${slug}-${counter}`;
          counter++;
      }
      fileSet.add(uniquePath);
      return React.createElement(
          `h${level}`,
          { id: uniquePath},
          React.createElement('a', {
            href: `#${uniquePath}`,
            key: `link-${uniquePath}`,
            className: 'anchor mdx-header-anchor',
          },children)
      );
    };
}
talatkuyuk commented 7 months ago

Do you use it in app folder or pages folder? next-mdx-remote doesn't have any cache mechanism, but Nextjs has in app router. I suppose your issue is not related with next-mdx-remote.

The fileSet is useless because the heading element can not know the others' fileSet value, which is scoped variable. Each heading element creates its own empty set.

talpx0 commented 7 months ago

Do you use it in app folder or pages folder? next-mdx-remote doesn't have any cache mechanism, but Nextjs has in app router. I suppose your issue is not related with next-mdx-remote.

The fileSet is useless because the heading element can not know the others' fileSet value, which is scoped variable. Each heading element creates its own empty set.

I think you are correct , I use next 14 and app router , everytime I refresh , the fileSet will become a closure (it didn't get memory free), and it will keep update the uniquePath = ${slug}-${counter} like : a-1 -> refresh -> a-2 -> refresh -> a-3 , I only use this for heading on current page only , So , It only checks the current path's md content , thanks

talatkuyuk commented 7 months ago

And, one more thing. You don't need to implement the unique url, fileset etc. There are some remark plugins to do so for you.