Conversation
|
we are waiting for the merge to happen so we can update our code base with yours, |
| "node-sass": "^4.5.3", | ||
| "node-static": "^0.7.9", | ||
| "nodemon": "^1.11.0", | ||
| "rollup": "^0.55.0", |
There was a problem hiding this comment.
I think rollup is necessary to build the bundled version (c3.js).
There was a problem hiding this comment.
It must be global because in the scripts where it's called you are using the global rollup command not from node modules, anyways i will add it back
| @@ -0,0 +1,13 @@ | |||
| { | |||
There was a problem hiding this comment.
Because babel is configured in rollup.config.js (when bundling) and karma.conf.js (when testing), I think this file is not necessary and confusing.
There was a problem hiding this comment.
For me the build keeps returning an error from rollup until i added this file
There was a problem hiding this comment.
Do you use npm run command for invoking rollup?
npm run build command should invoke rollup (and other tools) with correct configs, like:
$ npm run build
> c3@0.4.18 build /Users/kt3k/t/c3
> npm run build:js && npm run build:css
> c3@0.4.18 build:js /Users/kt3k/t/c3
> npm run build:js:rollup && npm run build:js:uglify
> c3@0.4.18 build:js:rollup /Users/kt3k/t/c3
> rollup -c > c3.js
(!) Some options have been renamed
https://gist.github.com/Rich-Harris/d472c50732dab03efeb37472b08a3f32
entry is now input
moduleName is now output.name
format is now output.format
...npm run command automatically uses local rollup cli and you don't need the global one.
There was a problem hiding this comment.
yes, I do use this, but the issue is not rollup, rollup works but throws a different error (It looks like your Babel configuration specifies a module transformer), so I added this file and it worked for me, anyways I removed this file for you to merge.
when will you publish the next version to npm? we are still using our own library.
|
Thanks for finding out these! 👍 |
Codecov Report
@@ Coverage Diff @@
## master #2273 +/- ##
==========================================
+ Coverage 73.88% 75.39% +1.51%
==========================================
Files 51 51
Lines 4185 4186 +1
==========================================
+ Hits 3092 3156 +64
+ Misses 1093 1030 -63
Continue to review full report at Codecov.
|
|
is there any update on this? Would love to see it go into master. |
|
@Mobiletainment you can use alooma-c3 while they release the new version, after they release it you simply change back to c3 in your package.json |
|
The root problem was that the attached function (resizeIfElementDisplayed) and the detached one (resizeFunction) were different and the attached one was leaked every time. This seems to have started at #2164. Thanks again for finding out this! By the way $$.resizeFunction.remove() doesn't seem removing internal resize functions, because it removes the only given function (see Lines 988 to 995 in 7e5c20c |
|
@kt3k great, this is the problem that I solved, hopefully not introducing another one.. |
4 participants
the detach event listener was not detached properly after calling .destroy
so I basically added resizeIfElementDisplayed to $$ (this), and detach the event listener properly
also .remove must be called on $$.resizeFunction on destroy,
anyways this fixed all my memory issue,
Fix #926