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

Fetch most dependencies via NPM #377

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

tinogo
Copy link

@tinogo tinogo commented Jan 29, 2019

This MR will unversion most of the bundled libs (except those libs, which have some kind of customization...) and load them via NPM instead.

This makes it possible to considerably cut down the size of the dist package and this also works around a vulnerability of the jQuery-File-Upload plugin, as the PHP connector/adapter isn't versioned anymore! :)

I hope it haven't broke anything?! :)

@KevVerF
Copy link

KevVerF commented Apr 24, 2019

I had some issues getting this to work:

  • src/scss/codemirror.scss => I had to remove the .css extension before grunt would include the actual content of the file
  • In the gruntfile I had to:
    • import node-sass "on top" => const sass = require('node-sass');
    • add "implementation" option => sass: { options: { implementation: sass } }
    • Disable the mangle option (it didn't check which dependency was out of scope) => uglify: { options: { mangle: false } }

I also updated packages.json, so that might be causing some of the above issues (especially in the gruntfile config)

{
"devDependencies": {
    "grunt": "^1.0.4",
    "grunt-contrib-concat": "^1.0.1",
    "grunt-contrib-copy": "^1.0.0",
    "grunt-contrib-cssmin": "^3.0.0",
    "grunt-contrib-uglify": "^1.0.1",
    "grunt-sass": "^3.0.2",
    "node-sass": "^4.11.0",
    "npm-run-all": "^4.0.2",
    "uglify-js": "^3.0.27"
  },
  "dependencies": {
    "@allmarkedup/purl": "git+https://github.com/allmarkedup/purl.git",
    "blueimp-file-upload": "^9.12.1",
    "blueimp-load-image": "^2.6.1",
    "blueimp-tmpl": "^3.3.0",
    "clipboard": "^2.0.4",
    "codemirror": "^5.35.0",
    "globalize": "^1.3.0",
    "highlightjs": "^9.12.0",
    "jquery": "^3.4.0",
    "jquery-contextmenu": "^2.0.1",
    "jquery-file-download": "^1.4.6",
    "jquery-mousewheel": "^3.1.13",
    "jquery-splitter": "git+https://github.com/GerHobbelt/splitter.git",
    "knockout": "^3.4.0",
    "malihu-custom-scrollbar-plugin": "^3.1.3",
    "markdown-it": "^8.2.2",
    "markdown-it-footnote": "^3.0.1",
    "markdown-it-replace-link": "^1.0.0",
    "pyrsmk-toast": "^2.2.0"
  }
}

At first glance all seems to work for us.

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.

None yet

2 participants