Auto Bundles were not working in AMD Environment #263
Conversation
If you mean we should just set window.ES6Promise then no, we should not. |
👍
Good catch. |
|
I'm trying to replicate the above test failure locally, but without much luck... |
|
Same here. I was pretty sure they were working when I opened my PR. Damn. |
|
I'm rebuilding master on travis, lets see if its node 4 also fails. If so, then maybe its some transitive? I really wish I could replicate locally... |
|
Ya, it failed again. I really wish it was easily replicatable. I am at work right now, I'll have to take deeper look later... |
Brocfile.js
Outdated
| return new Rollup(es5, { | ||
| rollup: { | ||
| entry: 'lib/' + entry, | ||
| cache: 'BUILD:es6-promise', |
There was a problem hiding this comment.
We have two rollups with almost every file repeated so to reuse the same cache (of file reads, AST etc.) for both of them.
https://github.com/rollup/rollup/wiki/JavaScript-API#cache
There was a problem hiding this comment.
ok I'm not doing it correctly.
There was a problem hiding this comment.
https://github.com/chadhietala/broccoli-rollup/blob/master/src/index.js
Ok just saw this plugin automatically does it for me. I was surprised why was I getting good speeds, the plugin was overriding the cache variable for me. 😜
There was a problem hiding this comment.
chadhietala/broccoli-rollup@5aaf8f3
That was added by you. I feel like such an idiot.
There was a problem hiding this comment.
@azizhk haha no worries, I thought maybe you found some better caching strategy hidden deep inside rollup itself (you got my hopes up 
).
|
Im really not sure whats up. I have tested this thoroughly locally in node 4 (the failing test) without any issues.. Going to merge this. |
|
moving node 4 to allowed failures for now: #272 Will test node 4 locally to confirm until we resolved the node 4 issue. |
This was because of this code:
es6-promise/dist/es6-promise.auto.js
Lines 11 to 12 in f5f44e8
ES6Promisewould not be defined on global and we would get an error saying, "Cannot read property polyfill of undefined".Though there are multiple workarounds, I thought this would be the best way to fix this.
I would have preferred to export nothing, just call the polyfill function, which would make it inline in the minified version, but that would break semver.
Also I would love to change the module name from ES6Promise to es6-promise, to keep it the same in NPM and on the browser. (But that would make it
window['es6-promise']for non AMD 😢 )Also this is my first time with
Broccoli.js. So please correct me if I'm doing something wrong.Also I did not commit all of the dist files. Let me know if I should not commit them.