search

Julusian / node-midi

A node.js wrapper for RtMidi providing MIDI I/O
https://www.npmjs.com/package/@julusian/midi
MIT License
22 stars 7 forks source link

Update RtMidi to latest #11

Closed beaugunderson closed 7 months ago

beaugunderson commented 8 months ago

I tested this locally on an M2 MBP running 14.1.2 and everything worked as expected.

beaugunderson commented 7 months ago

ah you probably don’t want the yarn upgrade portion of this! sorry, didn’t realize I had pushed that… if you revert those last two commits and just keep the rtmidi one you should be good

On Mon, Jan 15 2024 at 14:24, Julian Waller @.***> wrote:

Merged #11 https://github.com/Julusian/node-midi/pull/11 into master.

— Reply to this email directly, view it on GitHub https://github.com/Julusian/node-midi/pull/11#event-11492227709, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAPCXZVLZ2UHWSVGB47XY3YOWNB3AVCNFSM6AAAAABAT3BJGSVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGQ4TEMRSG43TAOI . You are receiving this because you authored the thread.Message ID: @.***>

beaugunderson commented 7 months ago

(i found that using yarn v4 in a project that pulls in this module results in this module not working on older macos or at all on windows… so i went back to yarn v1 in that project since i could never figure out why. but the new rtmidi works flawlessly)

On Mon, Jan 15 2024 at 15:13, Beau Gunderson @.***> wrote:

ah you probably don’t want the yarn upgrade portion of this! sorry, didn’t realize I had pushed that… if you revert those last two commits and just keep the rtmidi one you should be good

On Mon, Jan 15 2024 at 14:24, Julian Waller @.***> wrote:

Merged #11 https://github.com/Julusian/node-midi/pull/11 into master.

— Reply to this email directly, view it on GitHub https://github.com/Julusian/node-midi/pull/11#event-11492227709, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAPCXZVLZ2UHWSVGB47XY3YOWNB3AVCNFSM6AAAAABAT3BJGSVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGQ4TEMRSG43TAOI . You are receiving this because you authored the thread.Message ID: @.***>

Julusian commented 7 months ago

ah you probably don’t want the yarn upgrade portion of this!

no its fine, yarn 1 is EOL but I haven't gotten around to updating a lot of projects yet. I would have gotten to it myself eventually

(i found that using yarn v4 in a project that pulls in this module results in this module not working on older macos or at all on windows… so i went back to yarn v1 in that project since i could never figure out why.

yeah I cant explain why that would happen.. it shouldn't make any difference, yarn doesnt have to do anything interesting/special when installing this..

beaugunderson commented 7 months ago

i think it’s also going to prevent people using this module from being able to use it on yarn 1 since they’ll get a corepack error

On Mon, Jan 15 2024 at 15:24, Julian Waller @.***> wrote:

ah you probably don’t want the yarn upgrade portion of this!

no its fine, yarn 1 is EOL but I haven't gotten around to updating a lot of projects yet. I would have gotten to it myself eventually

(i found that using yarn v4 in a project that pulls in this module results in this module not working on older macos or at all on windows… so i went back to yarn v1 in that project since i could never figure out why.

yeah I cant explain why that would happen.. it shouldn't make any difference, yarn doesnt have to do anything interesting/special when installing this..

— Reply to this email directly, view it on GitHub https://github.com/Julusian/node-midi/pull/11#issuecomment-1892827543, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAPCX3EIEBXRGJNJKHNQXDYOWUBDAVCNFSM6AAAAABAT3BJGSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJSHAZDONJUGM . You are receiving this because you authored the thread.Message ID: @.***>

Julusian commented 7 months ago

i think it’s also going to prevent people using this module from being able to use it on yarn 1 since they’ll get a corepack error

I am pretty certain that is not the case. I have some other libraries where they are using yarn 4 in applications using yarn 1 (and some the other way around too). The only time I am aware it will matter, is it makes doing a yarn link harder, but not impossible. It may result in setting up the link manually

beaugunderson commented 7 months ago

unfortunately it does break yarn v1, I just upgraded to 3.2.0 and verified:

[write] 9:34:53 AM dist-electron/electron-env.d.js
[write] 9:34:53 AM dist-electron/main/index.js
[write] 9:34:53 AM dist-electron/main/midi-manager.js
[write] 9:34:53 AM dist-electron/preload/index.js
  • electron-builder  version=24.9.1 os=23.2.0
  • loaded configuration  file=/Users/beau/p/m-editor/electron-builder.json5
  • writing effective config  file=release/0.0.70/builder-effective-config.yaml
  • rebuilding native dependencies  dependencies=@julusian/midi@3.2.0 platform=darwin arch=x64
  • rebuilding native dependency  name=@julusian/midi version=3.2.0
  ⨯ cannot execute  cause=exit status 1
error This project's package.json defines "packageManager": "yarn@4.0.2". However the current global version of Yarn is 1.22.21.

    Presence of the "packageManager" field indicates that the project is meant to be used with Corepack, a tool included by default with all official Node.js distributions starting from 16.9 and 14.19.
    Corepack must currently be enabled by running corepack enable in your terminal. For more information, check out https://yarnpkg.com/corepack.

                    command=/Users/beau/.nvm/versions/node/v21.5.0/bin/node /Users/beau/.nvm/versions/node/v21.5.0/lib/node_modules/yarn/bin/yarn.js run install
                    workingDir=/Users/beau/p/m-editor/node_modules/@julusian/midi
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
beaugunderson commented 7 months ago

never mind, after some reading it seems that all I needed was corepack enable; that seems to have taken care of the build error without changing any of the yarn versions, global or otherwise... sorry for the noise :)

will let you know if it affects where the module runs though

Julusian commented 7 months ago

unfortunately it does break yarn v1, I just upgraded to 3.2.0 and verified:

oh, I didn't think of that.. thats a bug in electron-builder though it is trying to recompile this library, but there is no reason for it to do that in almost all circumstances. unfortunately it does this because it was written to handle libraries which did need to be recompiled for electron, and there isn't a way to opt out of it doing that

beaugunderson commented 7 months ago

alright false alarm--the rebuilt version using electron-builder on yarn v1 does still work on Windows. I am confident that if I upgraded my local project to yarn v4 again the rebuilt version would break on Windows, but I don't think that's the fault of this package (since it did the same thing when this package was using yarn v1). thanks for the merge! 🙏

aunnop739 commented 7 months ago

@