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

remove extraneous docs from overridden Noise methods. #248

Merged
merged 1 commit into from
Jan 29, 2018

Conversation

Spongman
Copy link
Contributor

these docs in noise.js are duplicate docs for methods defined in the base class. i have removed them from the derived class because they are out of sync with the base definitions and are causing errors with the typescript definition build.

@therewasaguy therewasaguy merged commit f937e5f into processing:master Jan 29, 2018
@therewasaguy
Copy link
Member

thank you @Spongman!

@Spongman Spongman deleted the remove-overridden-docs branch January 29, 2018 05:09
@Spongman
Copy link
Contributor Author

Thank you!

@JTBrunderley
Copy link

JTBrunderley commented Mar 12, 2018

Hi @therewasaguy and @Spongman! I have run into this issue while using the p5.js 0.6.0 release from inside an angular5 js app. I understand I can resolve this issue manually by correcting the return type in the typescript definition file, so working with it locally is no issue. However, when I try to deploy my app to Heroku, it downloads the files through node during deployment and I don't have the ability to fix it there, so my deployments all fail. I see this was merged into master a few months ago, do you know when the version containing the fix will be released and available through node?

Thank you for the help and your work here!

@JTBrunderley
Copy link

JTBrunderley commented Mar 12, 2018

For anyone looking for a work-around until until this fix is included in an official p5 release:

I created a windows batch file to manually correct the Noise amp method's return type in the p5.d.ts file.

Current Incorrect Line: amp(volume: number|object, rampTime?: number, timeFromNow?: number): void

Corrected Line: amp(volume: number|object, rampTime?: number, timeFromNow?: number): AudioParam

Inside file_fix.bat:

@echo off &setlocal
setlocal enabledelayedexpansion

set "search= amp(volume: number|object, rampTime?: number, timeFromNow?: number): void"
set "replace= amp(volume: number|object, rampTime?: number, timeFromNow?: number): AudioParam"
set "textfile=%~dp0node_modules\p5\lib\p5.d.ts"
set "newfile=%~dp0node_modules\p5\lib\p5_1.d.ts"
(for /f "delims=" %%i in (%textfile%) do (
set "line=%%i"
set "line=!line:%search%=%replace%!"
echo(!line!
))>"%newfile%"
del %textfile%
rename %newfile% p5.d.ts
endlocal

I run this batch file from node's package.json postinstall script. This way the p5.d.ts file is corrected directly after it is installed:

"postinstall": ".\\file_fix.bat"

If you are deploying to Heroku, then your script needs to be a bash script instead of a windows bat file.

Inside file_fix.sh:

SCRIPT=$(readlink -f "$0")
SCRIPTPATH=$(dirname "$SCRIPT")
sed -i 's/ amp(volume: number|object, rampTime?: number, timeFromNow?: number): void/ amp(volume: number|object, rampTime?: number, timeFromNow?: number): AudioParam/' $SCRIPTPATH/node_modules/p5/lib/p5.d.ts

This will fix the file before Heroku starts building the app. You will also need to give Heroku permission to execute the script. You can do that in the package.json postinstall like so:

"postinstall": "chmod a+x ./file_fix.sh && ./file_fix.sh && ng build --aot -prod"

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.

3 participants