Skip to content

Consider to use musical.js as a dependency #99

Open

Description

@timaschew

I just saw that the code from musical.js is just copied into this repo https://github.com/PencilCode/jquery-turtle/blob/master/jquery-turtle.js#L3865

Why not use musical.js as a dependecy?
It can be already installed with npm via GitHub endpoint with this command:

npm i PencilCode/musical.js --save

Then just require('musical.js') or any other way, but this is the common way to do it.

Activity

  1. MarkusBordihn commented on Jan 12, 2016

    @MarkusBordihn
    Contributor

    Agree to this, it would help for the readability to split the different parts in seperate files.
    But it would also mean that the uncompiled version could not be easily used without nodejs.

  2. timaschew commented on Jan 12, 2016

    @timaschew
    Author

    But it would also mean that the uncompiled version could not be easily used without nodejs.

    Of course, just use browserify or any other bundler like webpack or rollup

  3. timaschew commented on Jan 12, 2016

    @timaschew
    Author

    musicaljs itself use this strategy already since my pull request was merged.
    here is the command to bundle the files for the browser: https://github.com/PencilCode/musical.js/blob/master/package.json#L13

  4. MarkusBordihn commented on Jan 15, 2016

    @MarkusBordihn
    Contributor

    @davidbau
    Please merge #100 so that I could start splitting jquery-turtle in different files for an better overview.

  5. davidbau commented on Jan 15, 2016

    @davidbau
    Member

    Hi Anton, by the way, I'd love for jquery-turtle.js to also use browserify

    • (and we could keep some sort dist/jquery-turtle.js updated for easy use
      as a single file). I am occupied with other projects but I'd be grateful
      if a contributor decided to factor jquery-turtle this way.

    David

    On Tue, Jan 12, 2016 at 4:30 PM, Anton Wilhelm notifications@github.com
    wrote:

    musicaljs itself use this strategy already since my pull request was
    merged.
    here is the command to bundle the files for the browser:
    https://github.com/PencilCode/musical.js/blob/master/package.json#L13

    —
    Reply to this email directly or view it on GitHub
    #99 (comment)
    .

  6. MarkusBordihn commented on Jan 17, 2016

    @MarkusBordihn
    Contributor

    I took a closure look at the current implementation and some steps are required to make it work.

    @timaschew
    Could you please correct the "main" entry in musical.js/package.json to the correct "src/index.js" file. NPM require('...') is expecting an nodejs file and not an compiled js file.

    @davidbau
    jQuery and browserify are not really working out of the box together, there are some work arounds, but maybe it would be better to switch from jquery to an similar npm package instead.

  7. davidbau commented on Jan 17, 2016

    @davidbau
    Member

    markus - for example use what instead of jQuery?

    I was thinking that it would be nice to use browserify to assemble
    jquery-turtle.js out of separate files, but still have it depend on jQuery
    as a global rather than via a normal browserify require('jquery'). In other
    words, when building a browserified jquery-turtle.js, I'd like it NOT to
    embed a specific version of jquery, but instead assume $ is defined
    globally.

    That's because, for educational reasons, I'd like student to have jQuery $
    (and lodash _) available as globals, instead of buried inside a private
    browserify dependency... Since that's how jQuery is used on almost all
    websites.

    I think this is a common desire, and it looks like there is a package to do
    it. Can we use this to manage the browserify dependency on jQuery?
    https://www.npmjs.com/package/browserify-global-shim

    David

    On Sun, Jan 17, 2016 at 5:19 AM, Markus Bordihn notifications@github.com
    wrote:

    I took a closure look at the current implementation and some steps are
    required to make it work.

    @timaschew https://github.com/timaschew
    Could you please correct the "main" entry in musical.js/package.json to
    the correct "src/index.js" file. NPM require('...') is expecting an nodejs
    file and not an compiled js file.

    @davidbau https://github.com/davidbau
    jQuery and browserify are not really working out of the box together,
    there are some work arounds, but maybe it would be better to switch from
    jquery to an similar npm package instead.

    —
    Reply to this email directly or view it on GitHub
    #99 (comment)
    .

  8. MarkusBordihn commented on Jan 17, 2016

    @MarkusBordihn
    Contributor

    @davidbau
    For example there is CheerioJs (https://github.com/cheeriojs/cheerio) which provide similar options but is smaller as the jQuery library. But I don't know if it supports all of your needed manipulation.

    If you only want to use browserify to combine the JavaScripts you could also use the Closure Compiler without the need of having all code "nodejs" compatible.

    It is also possible to install the Closure Compiler over npm so the setup would be easy.

    But in the end its your decision if you want to use the nodejs way or an more general way. ;)

  9. timaschew commented on Jan 17, 2016

    @timaschew
    Author

    @MarkusBordihn

    Could you please correct the "main" entry in musical.js/package.json to the correct "src/index.js" file. NPM require('...') is expecting an nodejs file and not an compiled js file.

    Yes, that's true. I was not sure about that, because this has some impact, but of course this is the the way how to do it.

    jQuery and browserify are not really working out of the box together

    Of course it's working, see here an online demo: http://requirebin.com/?gist=2378b26f6453d37f4bc6


    If you only want to use browserify to combine the JavaScripts you could also use the Closure Compiler without the need of having all code "nodejs" compatible.

    It is also possible to install the Closure Compiler over npm so the setup would be easy.

    I'm -1 for that, because tests, especially unit test and even headless browser tests, can't be run then anymore, because they need nodejs.
    Moving from npm is also IMO the wrong direction because it's the de facto standard package manager for JavaScript.


    Dealing with dependencies like jquery etc. is sometimes tricky. It depends on what this library (jquery-turtle) should be. Should it be just a library which can't be used standalone? Or are there some standalone use cases?
    The decision for this results into either just provide some source files of jquery-turtle and package.json with dependencies (jquery, lodash, etc). Then there is no need to bundle a file, because this is done by the enduser/dev.
    But this is only true if you provide jquery-turtle in the CommonJS module style. It can be used as npm module and bundled with any tool, which rely on npm and most of the bundler tools support npm.

    But if you decide to support also AMD or none module system (just use window/global pollution) then you need a bundler and you need to bundle your library in the Universal Module Definition (UMD) style.

    But keep in mind CommonJS is the de facto standard while ES6 Module System will be implemented by the browsers in the next months/years. AMD is outdated and global pollution is discouraged.

    If you want provide jquery-turtle as a standalone version as well (for instance to show some examples in a simple HTML page) then you also need to bundle a standalone file. (In that case you don't need any UMD) So in that case you can just provide an additionally standalone file (in most cases in a dist directory, where also a minified file is located) alongside with the CommonJS file.


    @davidbau

    Can we use this to manage the browserify dependency on jQuery?
    https://www.npmjs.com/package/browserify-global-shim

    This is the description of the module:

    Are you using browserify to convert a module that depends on lo-dash, jQuery, q or some other omnipresent library? And you don't want to include this dependency in your browserify-build because it's already part of your web app?

    This is totally the opposite what I wrote above. It doesn't make sense for me. Either let bundle the dependencies by the enduser or provide a standalone and bundle everything. This browserify plugin is needed if you want to do a mix of both and that's the reason why it doesn't make sense for me.

  10. MarkusBordihn commented on Jan 19, 2016

    @MarkusBordihn
    Contributor

    I created the first step for this. Please check my pull request is fine for everyone ?

    If it is fine, we could go to the second step and adding musical.js as dependency.

  11. MarkusBordihn commented on Jan 19, 2016

    @MarkusBordihn
    Contributor

    @timaschew
    My ideas was never to move away from NPM but I seeing NPM and NodeJs as separate technology.
    So you could also use NPM without using NodeJs style or syntax.
    Please take a look at #102 and let me know if this will work for you as well ?

  12. davidbau commented on Jan 19, 2016

    @davidbau
    Member

    @timaschew writes: Should it be just a library which can't be used standalone? Or are there some standalone use cases?

    There are two important use-cases today:
    (1) jquery-turtle.js used directly via <script src="dist/jquery-turtle.js"> as a jquery-plugin, that we can post for example on a CDN and that can just be used via a script tag. This is one of the simple ways beginners use jquery plugins, and jquery-turtle.js should work that way.
    (2) jquery-turtle as an NPM dependency with browserify. If we build jquery-turtle.js internally using browserify, then it would be natural, when building bundles that include jquery-turtle as a dependency, we should be able to list it as a dpendency in package.json and have it work cleanly.

    AMD is not as important right now because it is complex and it's going away, and ES6 modules aren't supported by browsers yet (and have a bit of a while to go still). But I think it's important to support the script-src use case.

  13. timaschew commented on Jan 19, 2016

    @timaschew
    Author

    I don't understand the two cases completely, I try to guess what you meant.

    2
    jquery-turtle as an NPM dependency: neither musicaljs nor jquery or any other dependency is included in the main file which refers to the source code of jquery-turtle only. Dependencies like jquery, musicaljs or lodash are just require() lines in the code. There is no need to use browserify or some other bundler here. This is part of the end user. The end user can use jquery-turtle via require('jquery-turtle').

    1
    jquery-turtle standalone will include everything expect jquery itself. The file is generated with browserify. The end user needs to embed it this way.

    <script src="jquery.js">
    <script src="jquery-turtle-bundle.js">
    

    So there is a precondition that jquery is available in the global scope.
    The enduser can also use the minified version of jquery-turtle, which is also provided/bundled in the release.

  14. timaschew commented on Jan 19, 2016

    @timaschew
    Author

    This is totally the opposite what I wrote above. It doesn't make sense for me. Either let bundle the dependencies by the enduser or provide a standalone and bundle everything. This browserify plugin is needed if you want to do a mix of both and that's the reason why it doesn't make sense for me.

    Okay, I was wrong, seems to be a valid use case for jquery

    BTW: I would recommend to use the officially mentioned plugin for shimming modules: https://github.com/substack/browserify-handbook#browserify-shim

  15. MarkusBordihn commented on Jan 19, 2016

    @MarkusBordihn
    Contributor

    btw: I would recommend to use the officially mentioned plugin for shimming modules: https://github.com/substack/browserify-handbook#browserify-shim

    Thanks, already done in #104.

  16. davidbau commented on Jan 19, 2016

    @davidbau
    Member

    jquery-turtle as an NPM dependency...
    Dependencies like jquery, musicaljs or lodash are just require() lines in the code.

    Yes.

    jquery-turtle standalone will include everything expect jquery itself.

    Yes.

    We agree on the two important use cases. Thanks for clarifying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions