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

How about adding a generate header(like CoffeeScript)? #380

Closed
tiye opened this issue Apr 14, 2013 · 9 comments
Closed

How about adding a generate header(like CoffeeScript)? #380

tiye opened this issue Apr 14, 2013 · 9 comments

Comments

@tiye
Copy link

tiye commented Apr 14, 2013

When I put my repo onto Github, it was recognized as a JS repo.
Actually it's writtern in CoffeeScript, with build.js generated by Browserify

How about adding a header which is similar to CoffeeScript's?

// Generated by CoffeeScript 1.6.2

jashkenas/coffeescript#1778

michaelficarra added a commit to michaelficarra/commonjs-everywhere that referenced this issue Apr 15, 2013
@ghost
Copy link

ghost commented Apr 20, 2013

You can already do this with cat and FIFO handles:

cat <(echo '// Generated by CoffeeScript 1.6.2') \
<(browserify -t coffeeify action/main.coffee -d) > build.js

This isn't the sort of thing isn't in scope for browserify. browserify doesn't know what coffee script is, it just provides a transform api for other modules.

@ghost ghost closed this as completed Apr 20, 2013
@tiye
Copy link
Author

tiye commented Apr 20, 2013

One more concern: if I add one line before that file by hand, will it break sourecemap?

@michaelficarra
Copy link

@substack: You've misunderstood the request. @jiyinyiyong was asking for browserify to mark the code it generates as generated by browserify.

@jiyinyiyong : Yes, it would if browserify generated real source maps. Yet another reason for browserify to work with a structured representation of JS (preferably a standard like the spidermonkey AST format). Asking people to do string manipulation of the output is insane.

@tiye
Copy link
Author

tiye commented Apr 21, 2013

Yeah, I think a header generated from browserify is more reliable.
Seems we have to pretend we are CoffeeScript generated code.
https://github.com/github/linguist/blob/master/lib/linguist/generated.rb#L95

@ghost
Copy link

ghost commented Apr 21, 2013

@michaelficarra That is really silly. Transforms work on strings. Strings are easy. If you want an AST just var ast = require('esprima')(src). Most transforms don't even use the AST for anything. If browserify gave you an AST it would mean that experiments that generate valid ASTs from other languages wouldn't work. At all. This is silly.

As for the other points, what is the benefit of adding a header comment to browserify builds? I don't understand the upside well enough to support this.

@michaelficarra
Copy link

what is the benefit of adding a header comment to browserify builds?

The same reason the CoffeeScript compiler adds it. So people (and programs) that read the output know that it was generated from other source files.

Transforms work on strings. Strings are easy.

And strings are lossy. They don't preserve information such as the source file/position through transforms. They are also much less useful to other tools. My browser bundler, commonjs-everywhere, supports taking input from arbitrary compile-to-JS languages, bundling, and running the program through a minifier, all while preserving source content/position information because all of these tools use a standard, structured JS representation as an IR. This means people can debug their CoffeeScript source files from minified browser bundles, with basically no extra work on my part. Arguments like "strings are easy" from maintainers of popular projects are why we can't have nice things. You should be more concerned with what is objectively the best solution for your users and the JS ecosystem as a whole.

@ghost
Copy link

ghost commented Apr 21, 2013

Browserify already has a very nice way of nesting inline source maps, which removes the need for this. Please stop adding noise to my issues feed. You are very uninformed about how browserify actually works and just seem to be hijacking my issues to promote your own project.

@thlorenz
Copy link
Collaborator

BTW to not break or fix source maps you have two options:

  • a) add the header in a way that no new line gets added i.e. /* my header */ original code on first line
  • b) use mold-source-map to decode inlined source maps, adjust all mappings (add line offset) and re-encode. the most generic transform should allow you to do this however you like

I agree that b) seems like a lot of work, but if you must have a header in the bundle file that no one will look at, you at least have this option.

@ghost
Copy link

ghost commented Dec 31, 2014

I was trying to do a simple builds with an attached version comment at the top using the following script

"scripts": {
  "build": "cat <(echo '// mypkg-x.y.z') <(NODE_ENV=production browserify -r ./client/main.js:mypkg | uglifyjs -cm) > dist/mypkg.min.js"
}

But npm run script will use sh which doesn't support <() as @substack suggested. To remedy this situation, I ended up with

"scripts": {
  "build": "{ echo '// mypkg-x.y.z'; NODE_ENV=production browserify -r ./client/main.mypkg | uglifyjs -cm; } > dist/mypkg.min.js"
}

If there's a better way to do this kind of thing without lots of other configuration/dependencies, I'm unaware. Anyone with +1 intelligence should chime in here and help us plebs out.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants