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

Fix for NPM usage #78 #89

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Fix for NPM usage #78 #89

wants to merge 8 commits into from

Conversation

Unsigno
Copy link

@Unsigno Unsigno commented Nov 20, 2018

There is a proposal to solve the issue #78.

I have made the least possible changes to keep the file structure and the project the same.

The webm-writer library requires the "fs" module on Node and it causes some issues using webpack, it can be fixed on the webpack config, but i think we can delete the dependency if we target browsers.

@SimonHFrost
Copy link

Hi @Unsigno

I checked out your PR and set target: 'node' in my webpack config. But I'm still getting require related errors. Should they be analysed at build time, and the required code inlined? The required files no longer exist after the build step is performed.

ERROR in ./node_modules/ccapture.js/build/CCapture.all.min.js
Module not found: Error: Can't resolve './download' in '/Users/simonfrost/Code/threejs-tapered-hair/node_modules/ccapture.js/build'
 @ ./node_modules/ccapture.js/build/CCapture.all.min.js 1:24146-24167
 @ ./js/app.js
 @ multi (webpack)-dev-server/client?http://localhost:8080 ./js/app.js

ERROR in ./node_modules/ccapture.js/build/CCapture.all.min.js
Module not found: Error: Can't resolve './gif' in '/Users/simonfrost/Code/threejs-tapered-hair/node_modules/ccapture.js/build'
 @ ./node_modules/ccapture.js/build/CCapture.all.min.js 1:24188-24204
 @ ./js/app.js
 @ multi (webpack)-dev-server/client?http://localhost:8080 ./js/app.js

ERROR in ./node_modules/ccapture.js/build/CCapture.all.min.js
Module not found: Error: Can't resolve './tar' in '/Users/simonfrost/Code/threejs-tapered-hair/node_modules/ccapture.js/build'
 @ ./node_modules/ccapture.js/build/CCapture.all.min.js 1:24114-24130
 @ ./js/app.js
 @ multi (webpack)-dev-server/client?http://localhost:8080 ./js/app.js

ERROR in ./node_modules/ccapture.js/build/CCapture.all.min.js
Module not found: Error: Can't resolve './webm-writer-0.2.0' in '/Users/simonfrost/Code/threejs-tapered-hair/node_modules/ccapture.js/build'
 @ ./node_modules/ccapture.js/build/CCapture.all.min.js 1:24224-24254
 @ ./js/app.js
 @ multi (webpack)-dev-server/client?http://localhost:8080 ./js/app.js

@Unsigno
Copy link
Author

Unsigno commented Dec 20, 2018

Hi @SimonHFrost , thanks for your report , i was pointing directly the source so i don't noted the issue .

I updated the entry point of the NPM module , so it should works now. For browser you can link the single files ( i keep the UMD pattern ) or use the "build" source.

I tried to touch as little code as possible , but if there are still problems I could add some transpiler to build the code .

Anyway using 'node' as target will throw error , since "webm-writer" is using "fs", you can try "node-async" or find a solution for your needs. But some of the modules used are designed to work in browsers.

Best regards Unsigno.

@chrisgervang
Copy link

Does this work? It would be great to get this fix published.

joohee-lgtm pushed a commit to joohee-lgtm/webrtc-with-filter that referenced this pull request Nov 10, 2019
  - ccapture.js 는 import 대신 외부 링크를 사용해야함
  - 참고) spite/ccapture.js#89 에서 이슈 수정중
@akella akella mentioned this pull request Jun 4, 2020
@XiyuZhai97
Copy link

I tested this one and it works well!

@optimuspaul
Copy link

maybe someone could merge and build? pretty please!

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.

None yet

6 participants