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

Does not work with Require.js #193

Open
incompl opened this issue Jul 6, 2018 · 5 comments
Open

Does not work with Require.js #193

incompl opened this issue Jul 6, 2018 · 5 comments

Comments

@incompl
Copy link
Contributor

incompl commented Jul 6, 2018

Require.js thinks that the video.js external in umd.rollup.config.js is a script itself and is trying to load it from current path. I'm not sure how to fix or work around this issue.

Example: http://jsfiddle.net/uy9680os/1/

@brandonocasey
Copy link
Contributor

I have a working example with contrib-ads here: https://jsfiddle.net/uy9680os/57/

@gkatsev
Copy link
Member

gkatsev commented Jul 12, 2018

I think the change here with external usage on rollup which puts in declared dependencies in AMD is a breaking change for users. Also, the requirement on using map for rjs is an unnecessary burden.

@brandonocasey
Copy link
Contributor

Yeah I think we need to keep this issue open, but for now there is a work-around.

@gkatsev
Copy link
Member

gkatsev commented Jul 16, 2018

So, looking into why this is happening and I think it's because rollup's globals is broken and potentially doesn't match our expectations.
The UMD bundle it generates will always require/define the dependencies whether globals is set or not but our expectations based on browserify and browserify-shim is that once something is set to global, it will always load from that shimmed location (i.e., global/window object).

A workaround/quick fix would be to use something like https://github.com/patrickhulce/rollup-plugin-shim or https://github.com/rollup/rollup-plugin-alias to shim video.js and other "globals" into small files/strings that just return window.videojs etc.

It would also be nice if we can figure out why globals isn't working in rollup and whether our expectations match rollup's or not.

@brandonocasey
Copy link
Contributor

#200

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

No branches or pull requests

3 participants