surikov / midi-sounds-react

ReactJS component for musical and MIDI applications
MIT License
130 stars 13 forks source link

Compatible with react-boilerplate? #1

Closed ShaneMcX closed 6 years ago

ShaneMcX commented 6 years ago

This is an awesome library and I really appreciate the time you've put into it and the samples. I got it working in a small project I made with CreateReactApp, but I'm having difficulty finding the right place to put the MIDISounds object in a react-boilerplate app.

In a fresh clone, I tried adding it to HomePage/index.js,

  // eslint-disable-line react/prefer-stateless-function
  render() {
    return (
      <div>
        <h1>
          <FormattedMessage {...messages.header} />
        </h1>
        <MIDISounds
          ref={ref => (this.midiSounds = ref)}
          appElementName="app" // matches ID of root div in index.html
          instruments={[3]}
        />
      </div>
    );
  }
}

Which resulted in:

screen shot 2018-04-02 at 6 09 41 pm

or App/index.js,

export default function App() {
  return (
    <div>
      <Switch>
        <Route exact path="/" component={HomePage} />
        <Route component={NotFoundPage} />
      </Switch>
      <MIDISounds
        ref={ref => (this.midiSounds = ref)}
        appElementName="app" // matches ID of root div in index.html
        instruments={[3]}
      />
    </div>
  );
}

which didn't work either before or after the Switch.

Then I generated a new component and tried it there,

  // eslint-disable-line react/prefer-stateless-function
  render() {
    return (
      <div>
        <MIDISounds
          ref={ref => (this.midiSounds = ref)}
          appElementName="app" // matches ID of root div in index.html
          instruments={[3]}
        />
      </div>
    );
  }
}

but that gave me the same error.

As you can see, in all cases I'm returning a single wrapping div. In all cases, simply removing the MIDISounds element fixes the issue. I suspect this is just my lack of knowledge, but I'm not sure what to try next.

ShaneMcX commented 6 years ago

Interestingly, I was able to simply copy midisoundsreact.js into my project and then use it as a component. Probably some pilot error involved here, but I'll leave this open for a while in case there's something more interesting going on.

surikov commented 6 years ago

As you can see, in all cases I'm returning a single wrapping div.

No. I don't see it.

In all cases, simply removing the MIDISounds element fixes the issue.

Read error message. This is exact case of your problem. You set wrong property. Assign appElementName to valid React element or null. Check your code and fix it. No one can help you if you don't show full code.

Clone https://github.com/surikov/midi-sounds-react-examples All those examples works well. Look to the source to learn how to use the midi-sound-react components.

Use https://github.com/surikov/webaudiofont for sound if you can't learn ReactJS.

ShaneMcX commented 6 years ago

No. I don't see it.

In all the code samples above, each render() returns a single containing div. That's all I was pointing out, because that error usually means you are returning more than one element.

Read error message. This is exact case of your problem. You set wrong property. Assign appElementName to valid React element or null. Check your code and fix it. No one can help you if you don't show full code.

LOL. If it was that simple, I wouldn't have asked. I thought maybe you'd see something I did wrong in the snippets I included or that perhaps you knew of an incompatibility with react-boilerplate. Note that I added a comment clearly stating that I assigned appElementName to a valid React element. Your examples use "root" as the ID, but react-boilerplate uses "app".

Use https://github.com/surikov/webaudiofont for sound if you can't learn ReactJS.

Not sure if this was intended to be as snarky as it sounds, but as I noted above, I have a working app that is based on CreateReactApp (which is, you know, a ReactJS app). The samples are great, but they are not based on react-boilerplate, which uses many more packages so it seems reasonable to guess there might be an incompatibility.

I'll provide a full example sometime soon unless I spot the problem. In the meantime, I've got midisoundsreact.js building directly in my app, which required some work to make it pass eslint. If you're interested, I can provide a fork of the lintable version.

surikov commented 6 years ago

I can't check your error without full code.

Just remove

appElementName="app"

ShaneMcX commented 6 years ago

Yeah, that makes no difference.

The code is just a clean react-boilerplate clone, with midi-sounds-react added, but here's a repo for you to take a look at: https://github.com/ShaneMcX/For-Srgy

surikov commented 6 years ago

Your version of app/containers/HomePage/index.js

 export default class HomePage extends React.PureComponent {
   render() {
     return (
       <h1>
         <FormattedMessage {...messages.header} />
         <MIDISounds ref={el => (this.midiSounds = el)} instruments={[3]} />
       </h1>
     );
   }
 }

fixed version

 export default class HomePage extends React.PureComponent {
   playTestInstrument() {   this.midiSounds.playChordNow(3, [60], 2.5); }
   render() {
     return (
    <div>
       <h1>
         <FormattedMessage {...messages.header} />        
       </h1>
      <p><button onClick={this.playTestInstrument.bind(this)}>Play</button></p>
      <MIDISounds ref={el => (this.midiSounds = el)} appElementName="app" instruments={[3]} />
    </div>
     );
   }
 }
surikov commented 6 years ago

I found bug with syling for new version or React. I will fix it.

surikov commented 6 years ago

All done.

Apply fix from comment above. Change version in package.json to "midi-sounds-react": "^1.2.52",

ShaneMcX commented 6 years ago

Thank you!