Added a default swf url #2533
Added a default swf url #2533
Conversation
|
Could we just default to the cdn url in the code? |
|
If we think it's worth it we could go back to that. I was trying to completely separate out the CDN logic from the core codebase. |
|
Just a comment from outside the box. ;-) I think it would be great to have the CDN as default fallback. Should be noted in the docs that this is relying on the CDN, but I think that most of the users would prefer having a default fallback which is already hosted over setting up their own one. |
| @@ -13,6 +13,7 @@ import FlashRtmpDecorator from './flash-rtmp'; | |||
| import Component from '../component'; | |||
| import window from 'global/window'; | |||
| import assign from 'object.assign'; | |||
| import pkg from '../../../package.json'; | |||
There was a problem hiding this comment.
@gkatsev can you check my sanity on this approach?
There was a problem hiding this comment.
Also, this does mean we include the whole package.json inside of the output. We probably want to use something like https://www.npmjs.com/package/browserify-versionify instead.
There was a problem hiding this comment.
yeah, I was hoping to avoid another one of those... but it is better
|
Updated with @gkatsev's suggestions and tested. Appears to be working as it was before. |
| // Otherwise this adds a CDN url. | ||
| // The CDN also auto-adds a swf URL for that specific version. | ||
| if (!options.swf) { | ||
| let protocol = ('https:' === window.location.protocol ? 'https://' : 'http://'); |
There was a problem hiding this comment.
why not just use a protocol relative url?
There was a problem hiding this comment.
Thanks. Was copy/pasting from an old version. Fixed.
|
LGTM |
fixes #2488