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

autoNumeric does not work with Browserify #388

Closed
jsmarsch opened this issue Jan 31, 2017 · 21 comments
Closed

autoNumeric does not work with Browserify #388

jsmarsch opened this issue Jan 31, 2017 · 21 comments
Assignees
Milestone

Comments

@jsmarsch
Copy link

We are using npm to manage our packages, and commonjs + browserify to handle module loading.
I get the following error during the browserify build:
(this is gulp executing browserify):

Process terminated with code 1.
events.js:160
      throw er; // Unhandled 'error' event
      ^
SyntaxError: Unexpected token (51:4) while parsing C:\git\Idi.Marketplace\Idi.Marketplace.Web\node_modules\autonumeric\src\autoNumeric.js while parsing file: C:\git\Idi.Marketplace\Idi.Marketplace.Web\node_modules\autonumeric\src\autoNumeric.js
    at DestroyableTransform.end [as _flush] (C:\git\Idi.Marketplace\Idi.Marketplace.Web\node_modules\insert-module-globals\index.js:96:21)
    at DestroyableTransform.<anonymous> (C:\git\Idi.Marketplace\Idi.Marketplace.Web\node_modules\through2\node_modules\readable-stream\lib\_stream_transform.js:115:49)
    at DestroyableTransform.g (events.js:291:16)
    at emitNone (events.js:86:13)
    at DestroyableTransform.emit (events.js:185:7)
    at prefinish (C:\git\Idi.Marketplace\Idi.Marketplace.Web\node_modules\through2\node_modules\readable-stream\lib\_stream_writable.js:465:12)
    at finishMaybe (C:\git\Idi.Marketplace\Idi.Marketplace.Web\node_modules\through2\node_modules\readable-stream\lib\_stream_writable.js:473:7)
    at endWritable (C:\git\Idi.Marketplace\Idi.Marketplace.Web\node_modules\through2\node_modules\readable-stream\lib\_stream_writable.js:485:3)
    at DestroyableTransform.Writable.end (C:\git\Idi.Marketplace\Idi.Marketplace.Web\node_modules\through2\node_modules\readable-stream\lib\_stream_writable.js:455:41)
    at DestroyableTransform.onend (C:\git\Idi.Marketplace\Idi.Marketplace.Web\node_modules\through2\node_modules\readable-stream\lib\_stream_readable.js:495:10)

Looks like an unexpected token while parsing autoNumeric.js

@AlexandreBonneau
Copy link
Member

AlexandreBonneau commented Jan 31, 2017

Hey @jsmarsch, could you please post a gist or a codepen with the files needed to reproduce this error, as well as the commands you are typing?

For information, autoNumeric is built with Webpack. We are therefore not experts in Browserify!

@AlexandreBonneau AlexandreBonneau added the 💁‍♂️ Need reporter feedback A response is needed from the reporter before being able to do more label Jan 31, 2017
@jsmarsch
Copy link
Author

jsmarsch commented Feb 1, 2017

Sorry -- I'm not a browserify expert either :( I want to help, but I'm facing a delivery deadline of my own right now, and I just don't have the free moments to set up a full build environment with gulp, browserify, npm, tsify, and get it all working (it took a couple of weeks the first time!)

@AlexandreBonneau
Copy link
Member

No problem, whenever you feel ready to help us debug this one, we'll be here!
By the way, have you tried again with the latest 2.0.8 version?

@bs-thomas
Copy link

Not sure if my comment helps, I had a similar problem. I noticed the "main" key in the package.json object targets the "src/autoNumeric.js". So I didn't load the default one, but instead loaded the dist one. That worked out of the box on browserify.

@AlexandreBonneau
Copy link
Member

Thanks for your comment @bs-thomas, it does help to know there is a workaround, and perhaps even pinpoint why this is not working.
We'll investigate.

@AlexandreBonneau
Copy link
Member

@jsmarsch, @bs-thomas, is this problem resolved with the latest version in next?

Much has changed since 2.0.7 (including the target in the package.json file), and would like to know if that fixed your problem, thanks!

@bs-thomas
Copy link

@AlexandreBonneau Actually I'm using 2.0.8 when my problem happened.

Thus, I did this in my "browser" key in my package.json in order to resolve the issue. So for sure, this was still happening in 2.0.8.

  "browser": {
    "autonumeric": "./node_modules/autonumeric/dist/autoNumeric.js",
  },

@AlexandreBonneau AlexandreBonneau changed the title autoNumeric 2.0.7 does not work with browserify autoNumeric 2.0.8 does not work with browserify Mar 19, 2017
@AlexandreBonneau
Copy link
Member

AlexandreBonneau commented Mar 19, 2017

@bs-thomas unfortunately I'm not sure how I could correct that behavior for Browserify on our end.
Glad you were able to find a workaround though.

Was that working at any point since we switched to webpack?

Would it also be possible to try with your setup if the latest v4.0.0-beta* pose the same problem? That would be great!

@whiskeysauer
Copy link

FYI.. I just installed beta 4.0.0 via NPM and required it in my Browserify project. I did initially get the error message shown in this thread. I added the following "Browser" element though in package.json and that got rid of the error (per @AlexandreBonneau ). thx!

I am seeing one problem though that I have not been able to fix. I can initialize AutoNumeric on a class of elements. I have not been able to do an update. I tried some of the syntax I saw in a ES6 fix. Here's my code...
anElement = new AutoNumeric.multiple('.currency', { currencySymbol : '$' });
That one works. It updates multiple elements.

Then.. to do the update.. I am using this..
anElement.update({ currencySymbol : '$' });

This gives me... "anElement.update is not a function"

Does anyone know how to make an update work? If I do another init, of course I get the JS warning message. Thanks.

@AlexandreBonneau
Copy link
Member

AlexandreBonneau commented Apr 11, 2017

Hi @whiskeysauer, thanks for your input on that issue.

We'll have to see what side effects adding that browser line in package.json could have on other project configuration.

About your request on the AutoNumeric.multiple() function, please use Gitter or IRC next time for questions ;)

As you can see in the doc, AutoNumeric.multiple() always return an array of AutoNumeric objects. Which means that you should do something like:

anElementsArray = new AutoNumeric.multiple('.currency', { currencySymbol : '$' });
anElementsArray.forEach(anElement => {
    anElement.update({ currencySymbol : '$' });
});

Or, if you need all the given AutoNumeric-managed element to have the exact same option on initialization (you can change their option separately after), you could first initialize the first element with the options you want, then use the init() function to link other elements to it.
The handy thing when doing this, is that thereafter you only need one command to control all the linked element, for instance:

anElement1 = new AutoNumeric('.currency', { currencySymbol : '$' });
anElement1.init(domElement2)
          .init(domElement3)
          .init(domElement4);
anElement1.global.update({ options }); // Update the settings of each autoNumeric-managed elements
anElement1.global.unformat(); // etc.

I hope that helps.
Feel free to come by the chat if you have more questions!

@AlexandreBonneau
Copy link
Member

For info, the documentation about the browser field in package.json.

@AlexandreBonneau
Copy link
Member

I recognize that calling init() on multiple elements like that could be cumbersome @whiskeysauer, so I'll soon modify init() so that it also accepts an array of DOM elements, or a CSS selector :)

AlexandreBonneau added a commit that referenced this issue Apr 11, 2017
…OM elements, or a CSS selector as its first argument.

This has been discussed in issue #388 comments.

Signed-off-by: Alexandre Bonneau <alexandre.bonneau@linuxfr.eu>
@AlexandreBonneau
Copy link
Member

Ok, so to simplify things a bit, you can now directly use

anElement1 = new AutoNumeric(domElement1, { currencySymbol : '$' });
anElement1.init('.currency'); // This potentially selects multiple elements
anElement1.global.update({ options });
anElement1.global.unformat(); // etc.

// or use
anElement1.init([domElement2, domElement3, domElement4]); 

The documentation has been updated accordingly.

@munkychop
Copy link
Contributor

I've opened #467 which fixes the issue with Browserify.

@AlexandreBonneau in the next branch, is there any specific reason you target src/main.js instead of dist/autoNumeric.js?

@AlexandreBonneau
Copy link
Member

AlexandreBonneau commented Jul 21, 2017

Thanks for the PR @munkychop.

I changed the target to main.js since all the needed imports are done there.
However, this was indeed not needed in package.json since I already configured Webpack to use that file as the entry point (see ./config/webpack.config.base.js, the line entry: './src/main.js',).

So, since this is not needed and changing it back to dist/autoNumeric.js fix Browserify, let's use that then.
Thanks again!

@AlexandreBonneau
Copy link
Member

AlexandreBonneau commented Jul 21, 2017

One question though, shouldn't we target the non-minified version of the library?

@AlexandreBonneau AlexandreBonneau self-assigned this Jul 21, 2017
@AlexandreBonneau AlexandreBonneau added this to the v4.0.0 milestone Jul 21, 2017
@munkychop
Copy link
Contributor

@AlexandreBonneau in my PR I am targeting the non-minified version.

@AlexandreBonneau
Copy link
Member

Ah sorry, I meant shouldn't we target the minified version, in order to produce a smaller bundle?

@munkychop
Copy link
Contributor

Personally, I think it's better to target the non-minified version for setups like Browserify, because these kinds of projects are typically minified at build time. Targeting the non-minified version avoids double-minification.

@AlexandreBonneau
Copy link
Member

Ok, thanks for your input

@munkychop
Copy link
Contributor

👍🏽

AlexandreBonneau added a commit that referenced this issue Jul 21, 2017
Fix issue #388 autoNumeric does not work with Browserify
@AlexandreBonneau AlexandreBonneau changed the title autoNumeric 2.0.8 does not work with browserify autoNumeric does not work with Browserify Jul 21, 2017
AlexandreBonneau added a commit that referenced this issue Jul 21, 2017
…ility)

Signed-off-by: Alexandre Bonneau <alexandre.bonneau@linuxfr.eu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants