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

Cannot be used in Node 0.12 (browserify based apps) #15

Closed
ibc opened this issue Sep 29, 2015 · 18 comments
Closed

Cannot be used in Node 0.12 (browserify based apps) #15

ibc opened this issue Sep 29, 2015 · 18 comments
Labels

Comments

@ibc
Copy link

ibc commented Sep 29, 2015

/tmp $ npm install clipboard
clipboard@1.2.0 node_modules/clipboard
├── tiny-emitter@1.0.0
└── delegate-events@1.1.1 (component-event@0.1.4, closest@0.0.1)

/tmp $ node
> var C = require('clipboard')
/tmp/node_modules/clipboard/src/clipboard.js:1
(function (exports, require, module, __filename, __dirname) { import Clipboard
                                                              ^^^^^^
SyntaxError: Unexpected reserved word
    at exports.runInThisContext (vm.js:73:16)
    at Module._compile (module.js:443:25)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at repl:1:9
    at REPLServer.defaultEval (repl.js:132:27)
    at bound (domain.js:254:14)

Obviously import is not implemented in Node 0.12 as it is not ES6. However I usually build my JS web apps as a Node project + browserify. Unfortunately the usage of import makes this unfeasible.

@zenorocha
Copy link
Owner

What if we declare main to point to dist/clipboard.js? https://github.com/substack/browserify-handbook#browser-field /cc @mauriciosoares

@mauriciosoares
Copy link
Contributor

damn, this browser-field might do the trick

I was trying to point main directly to the dist file, but there was a lot of "require" errors in the terminal, since all the required files are already bundled in the dist file.

This might be a solution 👍

@mauriciosoares
Copy link
Contributor

@zenorocha, tested here with my lib, and I think that will actually work.

will you work on that?

so_good

@zenorocha
Copy link
Owner

HAHAHAHAAH

Nope, can you test with Node and send a PR?

@mauriciosoares
Copy link
Contributor

Sure, I'll probably work on that tonight.

@mauriciosoares
Copy link
Contributor

Hey @zenorocha, just did a PR #19

I'll take a look in how I can improve the tests for requirejs and commonjs projects, in the meanwhile, this PR has tests for projects that uses browserify.

screen shot 2015-07-20 at 13 03 42

@zenorocha
Copy link
Owner

@ibc it should be fixed as of https://github.com/zenorocha/clipboard.js/releases/tag/v1.3.1

Could you test and let us know?

@ibc
Copy link
Author

ibc commented Oct 1, 2015

If I add the lib (master branch) into the package.json of my browserify based project, calling browserify on the project fails because babelify is required. It is included in the devDependencies of the lib, but those are not installed. babelify should be placed under dependencies. If so, it does work :)

@ibc ibc changed the title Cannot be used in Node 0.12 Cannot be used in Node 0.12 (browserify based apps) Oct 1, 2015
@zenorocha
Copy link
Owner

I moved to dependencies as requested ;)

@ibc
Copy link
Author

ibc commented Oct 1, 2015

browserify does not need to be added into dependencies ;)

@ibc
Copy link
Author

ibc commented Oct 1, 2015

I mean: is up to me whether I'm building a browserify based project or not. If so, when browserify "compiles" clipboard.js it will honor the "browserify" data in package.json, but browserify should not be placed in dependencies.

@zenorocha
Copy link
Owner

But you can't run babelify without browserify. I'm confused.

@zenorocha zenorocha reopened this Oct 1, 2015
@zenorocha zenorocha added the p1 label Oct 1, 2015
@ibc
Copy link
Author

ibc commented Oct 1, 2015

Example usecase:

  • I build a JS library named myApp.js intended for browser usage.
  • myApp.js is made using the Node's way (require('xxxx');, package.json, etc).
  • My deps are the node-debug module and clipboard.js (they are included in the dependencies field of my package.json).
  • myApp.js includes a Gruntfile or gulpfile with a browserify task to build the "browserified" library dist/myapp.js, so I add browserify into my devDependencies.

When I run my browserify task it takes the debug module and clipboard module. When browserifing clipboard browserify realizes that there is a browserify field in its package.json, so it honors those settings (in this case, it loads the babelify transform).

That's all. I won't use the browserify module within clipboard but the browserify module I set into my project's package.json. Hope it's clear now :)

@heldr
Copy link
Contributor

heldr commented Oct 7, 2015

Uhm...one approach is to create a clipboard.transpiled.js into dist folder and point it in "browser" field(I can do this with a PR using babel compiler as dev dependencie).

Then neither browserify nor babelify would be a main dependency, and the install would turn faster on NPM 3. Not to mention webpack and jspm could reach this file as well without any transpiler required.

@heldr
Copy link
Contributor

heldr commented Oct 7, 2015

However we have src/clipboard-action.js to be transpiled as well . Maybe create a folder dist/transpiled and point it in "browser" field. 😞

@heldr
Copy link
Contributor

heldr commented Oct 7, 2015

Example: heldr@b041513

drawback: Babel's _classCallCheck is repeated twice

@zenorocha
Copy link
Owner

Fixed in v1.5.3

@heldr
Copy link
Contributor

heldr commented Oct 28, 2015

nice! 😎

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

No branches or pull requests

4 participants