Skip to content

Conversation

ajstanley
Copy link
Contributor

What does this Pull Request do?

Drupal attempts to optimize JS code, which may cause failure if the code is already minified, as ours usually is. The remote implementation has always explicitly marked the Roblib code as minified, but local code was not.
This PR adds a configuration option so we can specify whether our code has been minified or not.

  • Related GitHub Issue: (link)

  • Other Relevant Links: (Google Groups discussion, related pull requests,
    Release pull requests, etc.)

What's new?

A in-depth description of the changes made by this PR. Technical details and
possible side effects.

  • Changes x feature to such that y
  • Added x
  • Removed y
  • Does this change add any new dependencies?
  • Does this change require any other modifications to be made to the repository
    (i.e. Regeneration activity, etc.)?
  • Could this change impact execution of existing code?

How should this be tested?

A description of what steps someone could take to:

  • Reproduce the problem you are fixing (if applicable)
  • Test that the Pull Request does what is intended.
  • Please be as detailed as possible.
  • Good testing instructions help get your PR completed faster.

Documentation Status

  • Does this change existing behaviour that's currently documented?
  • Does this change require new pages or sections of documentation?
  • Who does this need to be documented for?
  • Associated documentation pull request(s): ___ or documentation issue ___

Additional Notes:

Any additional information that you think would be helpful when reviewing this
PR.

Interested parties

Tag (@ mention) interested parties or, if unsure, @Islandora/committers

@ajstanley
Copy link
Contributor Author

Added to the schema, and defaulted to false in the library hook. Not strictly necessary; if no value has been set the minified flag will be ignored, but this makes intent a little clearer.

@seth-shaw-asu
Copy link
Member

@ajstanley, can I get testing instructions for this?

@ajstanley
Copy link
Contributor Author

@seth-shaw-asu Sure, it's easy to prove that it doesn't hurt anything, but a little harder to prove that it works.
Build a new executable with https://github.com/roblib/mirador-integration-islandora and install it per instructions. If you build with mode set to 'production' in webpack.config.js (the default), the resulting executable will be minified.
Now the weird part. If your code is minified, Drupal wants you to mark it as such in the library config, see https://www.drupal.org/docs/develop/creating-modules/adding-assets-css-js-to-a-drupal-module-via-librariesyml
If the code is not marked as minified, Drupal attempts to condense the code on its own, which can bork some React modules. If minified is set to true (as it is our remote implementation), everything is fine. If minified is not set to true for local implementation, it may or may not fail.

Install your new code, and configure it to minified in admin/config/media/mirador. Clear caches and view an image or two.

tldr; Drupal expects minified JS to be marked as such.

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

Successfully merging this pull request may close these issues.

3 participants