0xfe / vexchords

JavaScript Chord Charts
http://vexflow.com/vexchords/
MIT License
864 stars 106 forks source link

Issue with React build #10

Open jdaudier opened 5 years ago

jdaudier commented 5 years ago

Hi Mo:

I was able to use your lib locally for my React app (built using Create React App).

However, when I build it for production, I'm seeing this error.

I did a lot of research and tried a bunch of things but couldn't figure out what's wrong.

I'm hoping you can give me some guidance.

Repro steps:

  1. Clone this branch: https://github.com/jdaudier/unfretgettable/tree/vexchords
  2. Run yarn build
  3. Serve the build
    yarn global add serve
    serve -s build
  4. Try using the app
  5. You should see this error
0xfe commented 5 years ago

Hi -- I've seen this kind of issue come up with Vexflow/React. It's most likely the virtual DOM getting in the way. Instead of const chordDiagram = new ChordBox('#canvas', ...), can you try passing in the ref directly (prob this.canvasRef in your code.)

jdaudier commented 5 years ago

Hi @0xfe:

I tried what you suggested but still ran into the same issue.

What's odd is that it works locally for me, just not on the production build. Any other suggestions on what I else can try? I was so excited to try out your lib for my app. Thanks!

0xfe commented 5 years ago

Did it start working locally after the change I suggested? Or before too?

jdaudier commented 5 years ago

It was working locally from the start. I guess it has something to do with the prod build of React and your lib.

jdaudier commented 5 years ago

Hi Mo:

Is there anything I can do to help in solving this bug? I would love to use your library in my React app.

0xfe commented 5 years ago

Hi Joanne -- I spent some time digging into this and it looks like React really takes over the entire DOM, making it tricky to do the type of dynamic manipulation that we do here. I'm looking into https://github.com/tanem/svg-injector to see if it'll help.

jdaudier commented 5 years ago

Mo: Thanks for the update!

giacomorebonato commented 5 years ago

Hi, do you know the reason why this problem appears only in the production build?

jdaudier commented 5 years ago

@0xfe Do you mind explaining what you think the problem is? Maybe I can try to help in some way.

0xfe commented 5 years ago

My hypothesis:

React uses a Virtual DOM, which is a shadow in-memory structure that represents the DOM. This is ostensibly faster than manipulating the DOM directly, and is enabled only for production builds.

When render() is called on a component, it's only the virtual DOM that is updated -- since this is fast, this can happen frequently. Shortly after, the actual DOM is updated with only the diffs between the last snapshot of the virtual DOM.

So here, the SVG library used in in vexchords (svgjs) looks for actual DOM elements upon initialization, and for some reason, cares that they're real. This fails, and you see the error.

Two options that I can think of: 1) use react-friendly svg library, or 2) replace the library calls with injecting raw svg elements (there aren't a lot.)

jdaudier commented 5 years ago

@0xfe Do you think this SVGs into React components library might help? https://github.com/smooth-code/svgr

jdaudier commented 5 years ago

Any luck with this issue yet @0xfe? Just curious.

ShayanJavadi commented 4 years ago

I'm using this along with React + Gatsby and it's working fine. I have my const chord = new ChordBox('#guitar-diagram'); in my componentDidMount (using hooks but it's the same), and it's working in both my production and dev environments.

j127 commented 3 years ago

@ShayanJavadi is your code online anywhere?

I'm having the same problem in Gatsby 2 where it works in development but not when I build the site. I'm creating the diagrams in bulk.

I get this error message:

Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: object.

It also says "for the full message or use the non-minified dev environment for full errors and additional helpful warnings", but that doesn't help, because it works in development mode. I thought it might have something to do with Gatsby's prerendering, but the original issue used create-react-app. :thinking:

// scale-patterns.ts
import React from "react";
import { ChordBox } from "vexchords";

export type Note = [number, number, string?];

export interface Pattern {
    label: string;
    notes: Note[];
}

export function createScalePattern(
    ref: React.MutableRefObject<HTMLDivElement>,
    notes: Note[]
) {
    const chord = new ChordBox(ref.current, {
        width: 200,
        height: 240,
    });

    chord.draw({
        chord: notes,
    });
}

I have the code that creates the diagram in useEffect, which I guess is like componentDidMount.

import React, { useRef, useEffect } from "react";

import { createScalePattern, Pattern } from "../../fretboard/scale-patterns";

import "./styles.scss";

interface FretboardDiagramProps extends Pattern {
    className?: string;
}

const FretboardDiagram = ({ label, notes }: FretboardDiagramProps) => {
    const diagramRef = useRef(null) as React.MutableRefObject<HTMLDivElement>;

    useEffect(() => {
        createScalePattern(diagramRef, notes);
    }, []);

    return (
        <div className="fretboard-diagram">
            <h3 className="pattern-label subtitle">{label}</h3>
            <div ref={diagramRef}></div>
        </div>
    );
};

export default FretboardDiagram;

I also tried replacing the refs with dynamically generated ids but that gave the same error.

const FretboardDiagram = ({ label, notes }: FretboardDiagramProps) => {
    const id = flattenDeep(notes).join("_");

    useEffect(() => {
        createScalePattern(id, notes);
    }, []);

    return (
        <div className="fretboard-diagram">
            <h3 className="pattern-label subtitle">{label}</h3>
            <div id={`diagram_${id}`}></div>
        </div>
    );
};
j127 commented 3 years ago

Sorry, it turned out that it was another error in my code due to having an extra file in the pages directory that Gatsby didn't like during build. The code above works.

I made a quick working example with create-react-app here in case anyone else runs into difficulty.