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

Which is the proper way to work with npm modules and browserify? #1878

Closed
trigoesrodrigo opened this issue Feb 26, 2015 · 11 comments
Closed

Comments

@trigoesrodrigo
Copy link

Hi. I'm trying to use some of the SUI modules inside an application in which I'm already using Browserify and npm modules.

I found some problems while trying to get the modules to work.
So far I've tried four modules: semantic-ui-checkbox, semantic-ui-dropdown, semantic-ui-tab and semantic-ui-modal.

With semantic-ui-checkbox everything worked properly with the following code:

var $ = require('jquery');
var Checkbox = require('semantic-ui-checkbox');
$.fn.checkbox = Checkbox;

....

//In my Marionette.View onShow method
this.$el.find('.ui.checkbox').checkbox();

However, with the other three modules I've encountered problems:

  • semantic-ui-dropdown: just requiring it produced an error with browserify that goes away if I remove the underscore in this line, probably a bug. After fixing that, another error occurs because internally it is using the transition module. I've tried including the module before (without the need of manually doing $.fn.transition = require('semantic-ui-transition') because this time the module does it automatically -I think either all modules should do that or none, otherwise it could be misleading-). After trying that, the error is different and Uncaught ReferenceError: _module is not defined pops up in this line after I click the menu.
  • semantic-ui-tab: similar problems as semantic-ui-dropdown. In this case Uncaught TypeError: Cannot read property 'exports' of undefined pops up in line when I invoke `.tab() on the DOM elements.
  • semantic-ui-modal: apparently this module depends on dimmer and dimmer on transition, so I've tried requiring them before (in the same fashion as before $.fn.dimmer = require('semantic-ui-dimmer'). After doing that, Uncaught ReferenceError: _module is not defined pops up in line when I invoke the modal. It looks as the same error as with the dropdown module.

Am I doing something wrong? Are there any examples of using that modules in a browserify environment?

Thanks in advance.

@jlukic
Copy link
Member

jlukic commented Feb 26, 2015

The NPM releases are created dynamically from source using a build script. The _module change came from #1156.

You can check out what the script is doing here with these regex.

I haven't had much time testing single component releases with npm. It would be really helpful if you can provide any feedback that will help me update the regexp to patch issues you are encountering

If you are interested in patching with PR you can run the build script locally using admin: true in your semantic.json and

gulp "create components"
# or
gulp release

@trigoesrodrigo
Copy link
Author

I'll try to make them work and provide feedback, although it's my first app with browserify and I'm not confident of finding a good solution.

Until everything with the modules is sorted out, another way to make everything work easily is to use browserify-shim to expose jquery to semantic-ui in your package.json.

  "browserify": {
    "transform": [
      "browserify-shim"
    ]
  },
  "browser": {
    "sui": "./semantic/dist/semantic.js"
  },
  "browserify-shim": {
    "sui": {
      "depends": [
        "jquery:jQuery"
      ]
    }
  }

Then, in any view in the app you could just require('sui') and use the modules without problems. It's not as clean as just requiring the exact modules used but it gets the job done.

Thanks for your wonderful library and the amount of effort you're putting in it.

@sdimit
Copy link

sdimit commented Mar 27, 2015

Update: possible solution
Any progress on fixing this bug?

I'm experiencing the same issues with Uncaught ReferenceError: _module is not defined when using semantic-ui-popup.

I believe I have found the issue:
In popup.js

module.exports = function(parameters){
   ...
};
module.exports.settings = {
   ...
   escape = module.exports.settings.templates.escape
   ...
};

When generating index.js, the above module.exports.settings is matched as a settings reference here and replaced with '_module.exports.settings' here.
The issue is that _module is defined in module.exports = function(...), and hence not the scope of module.exports.settings = {

Possible solution:
Moving var _module = module; outside of both assignments to module.exports.
However, I am not sure if that will cause a regression to #1156 but I can confirm it works with my setup.
Changing these two regex from the build script here:

 export            :  'module.exports = function(parameters) {\n  var _module = module;\n',
 formExport        :  'module.exports = function(fields, parameters) {\n  var _module = module;\n',

to

 export            :  'var _module = module;\nmodule.exports = function(parameters) {',
 formExport        :  'var _module = module;\nmodule.exports = function(fields, parameters) {',

should do the trick I hope.

@jlukic
Copy link
Member

jlukic commented Mar 27, 2015

I'll take a look at this today

@jlukic
Copy link
Member

jlukic commented Mar 27, 2015

Will release with patch today.

@jlukic jlukic closed this as completed Mar 27, 2015
@jlukic jlukic added this to the 1.x Changes milestone Mar 27, 2015
@jlukic
Copy link
Member

jlukic commented Mar 27, 2015

Closing, will re-open if anyone sees any continuing issues

@chessels
Copy link

This patch causes a syntax error in https://github.com/Semantic-Org/UI-Site/blob/master/index.js (line 13).

/*!
 * # Semantic UI 2.0.7 - Site
 * http://github.com/semantic-org/semantic-ui/
 *
 *
 * Copyright 2015 Contributors
 * Released under the MIT license
 * http://opensource.org/licenses/MIT
 *
 */
;(function ( $, window, document, undefined ) {

$.site = var _module = module;
module.exports = function(parameters) {

@jlukic
Copy link
Member

jlukic commented Jul 26, 2015

I'm pretty sure automatically creating modules via regExp is a terrible monkeypatch. Sorry about these kinds of bugs.

@chessels
Copy link

I agree, the RegExp approach is not very solid. No need to appologise though.

I guess refactoring each component to have built-in support for UMD/CommonJS would be the proper solution.

@davide
Copy link
Contributor

davide commented Aug 9, 2015

Pull request 2816 (attached above) should take care of the issues described by @trigoesrodrigo and @chessels (as well as a similar issue with the api component).

@jlukic if everything looks good can you please do a new npm release for us? Thanks!

@jlukic
Copy link
Member

jlukic commented Aug 9, 2015

I'll take a look at this specifically for a patch monday. Still coralling things together for a larger 2.1 release with some new features possibly later in week.

jlukic pushed a commit that referenced this issue Aug 10, 2015
Fixes npm packaging for components api, site (#1878) and transition.
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