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

Consider to use musical.js as a dependency #99

Open
timaschew opened this issue Dec 10, 2015 · 16 comments
Open

Consider to use musical.js as a dependency #99

timaschew opened this issue Dec 10, 2015 · 16 comments

Comments

@timaschew
Copy link

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.

@MarkusBordihn
Copy link
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.

@timaschew
Copy link
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

@timaschew
Copy link
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

@MarkusBordihn
Copy link
Contributor

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

@davidbau
Copy link
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 [email protected]
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)
.

@MarkusBordihn
Copy link
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.

@davidbau
Copy link
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 [email protected]
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)
.

@MarkusBordihn
Copy link
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. ;)

@timaschew
Copy link
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.

@MarkusBordihn
Copy link
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.

@MarkusBordihn
Copy link
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 ?

@davidbau
Copy link
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.

@timaschew
Copy link
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.

@timaschew
Copy link
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

@MarkusBordihn
Copy link
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.

@davidbau
Copy link
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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants