europeana / media-player

Media player developed under the Europeana Media Generic Services Project
European Union Public License 1.2
13 stars 1 forks source link

Dependencies of production builds? #76

Closed rwd closed 4 years ago

rwd commented 4 years ago

It is not clear from the README what the dependencies are of a production build, when imported in an HTML script element.

This is exemplified by changing the Nightwatch runner Node env to "production", which I would argue is more representative in e2e tests of what is being tested:

diff --git a/nightwatch-runner.js b/nightwatch-runner.js
index 4888a97..240ea55 100644
--- a/nightwatch-runner.js
+++ b/nightwatch-runner.js
@@ -1,4 +1,4 @@
-process.env.NODE_ENV = 'development';
+process.env.NODE_ENV = 'production';
 const server = require('./webpack-dev-server.js');

 server.ready.then(() => {

There are then missing dependencies in the test fixture HTML file, resulting in all e2e tests failing, but what are those dependencies?

rwd commented 4 years ago

By way of further example, generate the bundle with:

NODE_ENV=production npm run build

Then in the dist/ directory, create an HTML file containing:

<!doctype html>
<html lang="en">
  <head>
    <meta charset="utf-8">

    <title>Europeana media player: production build</title>

    <script src="./europeana-media-player.min.js"></script>

    <style>
      #player {
        height: 640px;
      }
    </style>
  </head>

  <body>
    <div id="player"/>
    <script>
      let manifest = new URL(window.location.href).searchParams.get('manifest');
      if (!manifest) manifest = 'https://iiif.europeana.eu/presentation/2051926/data_euscreenXL_EUS_3C083B8925D2E14C954507769E47992A/manifest?format=3';

      const player = document.getElementById('player');
      new EuropeanaMediaPlayer(player, {
        manifest
      });
    </script>
  </body>
</html>

Open the HTML file and observe the console error:

europeana-media-player.min.js:1 Uncaught TypeError: e is not a function
    at t.init (europeana-media-player.min.js:1)
    at new e (europeana-media-player.min.js:1)

The question now is: what needs to be added to that HTML file in order to make the player operational?

pietervanleeuwen commented 4 years ago

As stated in the README - Dependencies the package has unbundled depencies on jQuery and jQuery-UI

You can see these being used in for example in this text fixture https://github.com/europeana/media-player/blob/develop/tests/fixture-data/index.html

rwd commented 4 years ago

Despite what the README states, adding jQuery and jQuery-UI (in the following manner) to the HTML document in my previous comment does not resolve this issue:

<script src="https://code.jquery.com/jquery-3.4.1.min.js" type="text/javascript"></script>
<script src="https://code.jquery.com/ui/1.12.1/jquery-ui.min.js" type="text/javascript"></script>

As indicated in this issue's first post, the reason that the e2e tests pass (with only jQuery included, not jQuery-UI), is that the Nightwatch runner uses the development Node environment, not production.

So, again: what precisely would need to be added to the HTML document linking to a production bundle to get it working?

pietervanleeuwen commented 4 years ago

Thanks for showing the includes, this was left out of your earlier code example.

It appears there was an issue with webpack externals and the provideplugin overriding the globals that jQuery exposes.

This issue has been addressed in PR #77 https://github.com/europeana/media-player/blob/a3bb335872a1091c068b8a5df371f0962a350b6a/webpack.config.js#L145-L168

rwd commented 4 years ago

Indeed. This is confirmed resolved on develop branch.