Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

When building modules are added twice or some times thrice #56

Closed
mailaneel opened this Issue · 6 comments

2 participants

@mailaneel

Issue 1

If i add more than one module name to build, in the production.js after the build is done i am getting file duplicates, text,i18n,order plugins are added more than once. text files and language file are also added more than once, I am also adding the build production.js file for reference, please scroll bottom to see the multiple additions

if i change the order in build file i am getting duplicates, just search for define('i18n' in the gist link, the production.js will have 2 "i18n" modules, production1.js will have 1 "i18n" module, the only difference is the module order in build file

Issue 2

If i add require inside define that is not combined during the build process, try loading html file in some function and build the file html file is not added to the build it is loaded using a separate request

Gist Link

Gist

Below is my build file

    ({
    appDir: "../",
    baseUrl: "web/",
    dir: "../../project-build",
    //Comment out the optimize line if you want
    //the code minified by UglifyJS
    optimize: "none",

    paths: {
        "order": "../common/js/vendor/requirejs/plugins/order",
        "text": "../common/js/vendor/requirejs/plugins/text",
        "i18n": "../common/js/vendor/requirejs/plugins/i18n",
        "wsl":"../common/js/vendor/wsl/base",
        "goog":"../common/js/vendor/goog",
        "bootstrap":"../common/js/vendor/bootstrap",
        "iscroll":"../common/js/vendor/iscroll",
        "jquery":"../common/js/vendor/jquery",
        "layout":"../common/js/vendor/layout",
        "mustache":"../common/js/vendor/mustache",
        "underscore":"../common/js/vendor/underscore/underscore",
        "spine":"../common/js/vendor/spine",
        "models":"../core/models",
        "web":"modules", // this is the base path for web

        //All modules should be configured here
        "web.config":"modules/config/config",
        "web.app":"modules/app/app",
        "web.actionBar":"modules/actionBar/actionBar",
        "web.sideBar":"modules/sideBar/sideBar"
    },

    modules: [
    //for some reasons if you add more than one module here during the build process modules are added twice and sometimes thrice
    // so if you want to make a build for production add dependencies in production.js file
    {
        name: "production"
    },
   {
        name: "web.sideBar"
    }
    ]
})

Main File

require.config({
    //what ever paths you mention here should be copied to build file
    paths: {
        "order": "../common/js/vendor/requirejs/plugins/order",
        "text": "../common/js/vendor/requirejs/plugins/text",
        "i18n": "../common/js/vendor/requirejs/plugins/i18n",
        "wsl":"../common/js/vendor/wsl/base",
        "goog":"../common/js/vendor/goog",
        "bootstrap":"../common/js/vendor/bootstrap",
        "iscroll":"../common/js/vendor/iscroll",
        "jquery":"../common/js/vendor/jquery",
        "layout":"../common/js/vendor/layout",
        "mustache":"../common/js/vendor/mustache",
        "underscore":"../common/js/vendor/underscore/underscore",
        "spine":"../common/js/vendor/spine",
        "models":"../core/models",
        "web":"modules",

        //All modules should be configured here
        "web.config":"modules/config/config",
        "web.app":"modules/app/app",
        "web.actionBar":"modules/actionBar/actionBar",
        "web.sideBar":"modules/sideBar/sideBar"
    },
    waitSeconds: 15,
    locale: "fr-fr"
});

require(
    [
    'wsl',
    'order!spine/spine',
    'order!spine/route',
    'order!spine/manager',
    'layout/layout',
    'iscroll/iscroll'
    ],function(){
         wsl.provide('web');
         wsl.provide('web.Config');
        //Start Application
        require(['order!web.config','order!web.app'], function(config,App){ 
            web.Config = config;
            $(document).ready(function(){
                var app = new App();
                app.start();
            });

        }); 
    });


Production File

require(['main'],function(){
    //from here main config will be loaded so the urls will be changed according to the config

    require(['web.sideBar']);
    require(['web.actionBar']);
});


web.sideBar

define('web.sideBar',
    [
    'text!web/sideBar/html/sideBar.html!strip',
    'text!web/sideBar/css/sideBar.css',
    'i18n!web/sideBar/nls/i18n' 
    ],
    function(html,css,language){

        var SideBar = Spine.Controller.sub();

        SideBar.extend({
            module: {
                name: 'web.sideBar',
                css: css,
                lang: language
            }
        });

        SideBar.include({
            init: function(){
                this.render();
            },
            render: function(){
                console.log("I am render from Side Bar");
                var self = this;
                    var options = {
                        element: self.el,
                        tpl:html, 
                        module:SideBar.module
                    };
                    wsl.module.render(options);
            },
            bindEvents: function(){

            }
        });       
        return SideBar;
    });


web.actionBar

define('web.actionBar',
    [
    'text!web/actionBar/html/actionBar.html!strip',
    'text!web/actionBar/css/actionBar.css',
    'i18n!web/actionBar/nls/i18n'
    ],
    function(html,css,language){

        var ActionBar = Spine.Controller.sub();

        ActionBar.extend({
            module: {
                name: 'web.actionBar',
                css: css,
                lang: language
            }
        });

        ActionBar.include({
            init: function(){
                this.render();
            },
            render: function(){
                console.log("I am render from Action Bar");
                var self = this;
                    var options = {
                        element: self.el,
                        tpl:html, 
                        module:ActionBar.module
                    };
                    wsl.module.render(options);
            },
            bindEvents: function(){

            }
        });       
        return ActionBar;
    });



@mailaneel

This is happening when using version 1.0.2

@jrburke
Owner

For issue 1, where you see duplicates, if you mean that you see the modules show up in 'production.js' and 'ws.sideBar.js', use the exclude directive in the build file for ws.sideBar.js:

    modules: [
    {
        name: "production"
    },
   {
        name: "web.sideBar",
        exclude: ["production"]
    }

For Issue 2, dynamic require([]) inside a define() call will not be included unless the dependency is a string literal. So if the require looks like:

define(function(require) {
    //a should be included in the build
    require(['a'], function () {});

    var dep = 'b';
    //b will not be included in the build
    require([dep], function () {});
});

Note that for 1.0.3 by default r.js will not scan for require([]) calls inside a define, unless findNestedDependencies: true in the build config. This is also true if you happen to be using r-edge.js, which is a build of the latest code in the repo (contains all the fixed 1.0.3 issues so far).

If you are saying the require(['a'], function(){}) use above is not found in 1.0.2, if you can point to a specific code reference, or show the define() with the nested require([]) call that would be helpful in tracking down the issue.

@mailaneel

For issue 1, i am talking about i18n, order plugins being included twice or thrice and also text dependencies are included twice. And i understand the exclude directive but production is not defined as dependency for web.SideBar module, so why should we exclude it. And if you see the gist attached you can check the build file and resulted file.

I am not a expert in build process but below is my understanding, please correct me if i am wrong:

My assumption is if a file or plugin is included once in the build it should not be included again for example there should not be two or three define('i18n',function(){}); , but i am seeing a file being included 2 or 3 times, My understanding is if i use the build tool a file or dependency will be included only once even if multiple files or multiple modules define the same dependency.

Scenario

if i have 2 modules TopBar, SideBar, both have dependency on a Model, and say i have a file called production which depends on
TopBar and SideBar. Now i want to build 3 modules TopBar,SideBar,Production.

My assumption:
SideBar and TopBar build file should have Model file include in them
Production build file should have SideBar file, TopBar file and Model file

But the outcome is Production file has SideBar build file, TopBar build file which means Model file is included twice and this is not the desired behavior according to me

Example: Gist
In the above link please search for define('i18n you will see this module included 2 times

Specific Code as requested for last point in the comment above

If i use the require in the renderUI function it is not build please see below

define('web.app',
    [
    'text!web/app/css/app.css',
    'i18n!web/app/nls/i18n' 
    ],function(css,language){

        wsl.provide('web.app');  

        web.app = wsl.Controller.sub();

        web.app.extend({
            module: {
                name: 'web.app',
                css: css,
                lang: language
            }
        });

        web.app.include({
            events: {},
            elements: {},
            init: function(){
            },
            start: function(){
                this.bindEvents();
            },
            bindEvents:function(){
                $(document).ready(this.proxy(this.onReady)); 
            },
            onReady: function(){
                wsl.log('Application is ready to use','warn'); 
                this.renderUI();
            },
            //this one should also be changed according the application requirements
            renderUI: function(){
                require(['text!web/app/html/base.html!strip'],function(html){
                    var options = {
                        element: $('body'),
                        tpl: html,
                        module:web.app.module
                    };

                    wsl.module.render(options); 
                });

            }
        });

        return web.app;
    });

@jrburke
Owner

For issue 1, if you clear/delete the built output then do a build, do the modules only show up once then? For what it is worth, the files should only appear once in the file. I'm having trouble figuring out a way to reproduce the problem. If you have a zip file of the contents I could try, feel free to send it to me privately. I'm jrburke on gmail too.

For issue 2, I was able to reproduce, thanks to your code sample. I pulled out the issue as issue #60 and there is a fix in there for it. You can try r-edge.js to verify that fix.

@mailaneel

Event if i delete the output folder this is happening, And i also observed that i do not delete the output folder before doing a new build,
build is not correctly done.

As requested, i have sent an email

@jrburke jrburke closed this issue from a commit
@jrburke Fixes #56, a built layer's output could be picked up as part of anoth…
…er layer if the first layer is a dependency of the second one.
6486a15
@jrburke jrburke closed this in 6486a15
@jrburke
Owner

I looked more into this, and the problem is this: when the 'web.actionBar' module is optimized, it is written to disk with all of its dependencies inside it.This is done before the 'production' output is written to disk. The 'production' depends on 'web.actionBar' so when 'production' comes second in the list, it picks up the contents of the optimized web.actionBar, which includes the plugins.

So I did a fix to not write out the build layers to their final names until all the build layers have been written to disk at temporary names, then move them to their final position.

You can try r-edge.js to verify the fix. This will go into the 1.0.3 release.

Thank you very much for the test case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.