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

bump osc version #5

Closed
wants to merge 1 commit into from
Closed

bump osc version #5

wants to merge 1 commit into from

Conversation

tknobi
Copy link

@tknobi tknobi commented Apr 19, 2018

Hi @nomve,

can you merge this, please?

Bump osc version, otherwise you'll get in error during npm install while using node v8 lts.

Thanks to @dmdrmc, for providing this fix.

Cheers
Tobi

@nomve
Copy link
Owner

nomve commented Apr 19, 2018

Hi @knoblochtobias

I will see what I can do to fix the build and publish these changes

@tknobi
Copy link
Author

tknobi commented Apr 25, 2018

The build problem is caused by a problem in osc.js
I already created a pull request for this

@nomve
Copy link
Owner

nomve commented Apr 25, 2018

I think there are some more issues there, but I didn't check it out in detail. Unfortunately I don't really have the time -- when this osc PR is cleared, I'll see if it works to merge and publish regardless of the build here. would that work for you?

@tknobi
Copy link
Author

tknobi commented Apr 25, 2018

Yes thats ok, thank you.

@colinbdclark
Copy link

I've commented on @knoblochtobias' osc.js pull request, but thought I'd include a note here. The issue, as far as I can tell, is that Tuio.js is using the pre-compiled osc-browser.js distribution, but also Browserify. In general, it doesn't make sense to run pre-compiled dist files through something like Browserify—one typically uses them as is, or processes the source code.

In Tuio.js, I attempted to replace the require("osc/dist/osc-browser") calls with require("osc") so that Browserify will be processing the underlying source files instead of a prebuilt artefact. This helps Browserify not freak out, but Tuio.js' tests continue to fail for whatever reason.

Alternatively, perhaps there's some way to tell Browserify not to attempt to process certain files?

I hope this helps!

@nomve
Copy link
Owner

nomve commented Apr 25, 2018

Hi @colinbdclark

thanks for the feedback. I'd say there are two separate issues; I'll add a comment on importing the browser version in the other repo.

In this case: can you give a hint which file to import? this is used in a browser, so doing require('osc') (which is src/platforms/osc-node.js) doesn't make much sense because it imports modules that won't work in a browser. e.g.

var dgram = require("dgram"),
        SerialPort = require("serialport"),
        net = require("net"),
        WebSocket = require("ws"),

@nomve
Copy link
Owner

nomve commented Apr 25, 2018

@knoblochtobias I have basically fixed the build locally but I need to see which file to import. If your PR goes through it will work, if not, I will need to find a workaround.

@nomve
Copy link
Owner

nomve commented Apr 27, 2018

@knoblochtobias fixed with #6

I have publish 1.0.0 with the fixes

@nomve nomve closed this Apr 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants