angular-translate / angular-translate

DEPRECATED Translating your AngularJS 1.x apps
https://angular-translate.github.io
MIT License
4.34k stars 1.05k forks source link

Why use AMD? #1099

Closed vance closed 9 years ago

vance commented 9 years ago

Why not just include ngTranslate as a normal angular module, like every other standard module? Why bother with AMD? It just breaks Karma/Jasmine/Require. Every ANGULAR lib I've tried to incorporate that uses AMD breaks Karma or Jasmine unless I delete the AMD stuff, then that breaks the submodule pattern to manually include them. This is on multiple projects, and I've heard other developers gripe about this.

I load everything through require config in my specs...

Am I missing something?

/*!

knalli commented 9 years ago

It just breaks Karma/Jasmine/Require.

No, not really. ;)

What you see is the UMD header which contains three things (it's actually explained in the code you have copied): AMD header, CommonJS and standard environment. Both AMD (i.e. RequireJS) and CommonJS are optional used so it will breaking nothing. Said this, this will not break neither Jasmine or Karma. Actually, Jasmin will not load modules.

These files can be loaded just like "regular" files and everything works as expected. You can see everything works because everyone use these files, as well on our own as in public plnkr/jsfiddle demo setups. In these situations like plnkr no AMD or CommonJS is used.

Perhaps you will tell us what exactly is going wrong?

vance commented 9 years ago

Mismatched anonymous defines. Don't get this with other libs. Only angular translate and ng-table, which support AMD. They are tagged included in Karma (still loaded just with HTML), but still fail. I've seen this happen with several projects with different configs and environments.

Below is the jasmine 2 config for grunt-contrib-jasmine and the karma.conf for Karma tests (running in phantom)

jasmine: { src: [ './src/main/app/*/[^(.Int)].js', './src/main/commons/*/[^(.Int)].js', './src/templates.js', ], options: { keepRunner: true, build: true, vendor: [ //'src/main/resources/lib/jjv.js', 'src/bower-lib/jquery/jquery.js', 'src/main/resources/lib/nanoScroller.js', 'src/bower-lib/angular/angular.js', 'src/bower-lib/angular-mocks/angular-mocks.js', 'src/bower-lib/angular-resource/angular-resource.js', 'src/bower-lib/angular-route/angular-route.js', 'src/bower-lib/angular-loader/angular-loader.js', 'src/bower-lib/angular-cookies/angular-cookies.js',

                'src/main/resources/lib/debugger/dimple.v1.1.2.min.js',
                'src/main/resources/lib/Stacktrace.js',

                //'src/bower-lib/stacktrace-js/dist/stacktrace.js',

                'src/main/resources/lib/angular-translate.js',
                'src/main/resources/lib/angular-translate-loader-static-files.js',

                'src/main/resources/lib/i18n/angular-locale_en-us.js',
            ],
            // we don't need this now that specs are in src
            //specs: 'src/**/*.Spec.js',
            helpers: 'src/**/*.Helper.js',
            host: 'http://127.0.0.1:8000/',
            template: require('grunt-template-jasmine-requirejs'),
            templateOptions: {
                requireConfig: {
                    baseUrl: '.grunt/grunt-contrib-jasmine/',
                    paths: {
                        // External libraries
                       // 'jjv': 'src/main/resources/lib/jjv.js',
                        'jQUery':   'src/bower-lib/jquery/jquery',
                        'nanoScroller': 'src/main/resources/lib/nanoScroller',
                        'angular': 'src/bower-lib/angular/angular',
                        'angularCookies': 'src/bower-lib/angular-cookies/angular-cookies',
                        'angularMocks': 'src/bower-lib/angular-mocks/angular-mocks.js',
                        'angularResource':  'src/bower-lib/angular-resource/angular-resource',
                        'angularRoute':  'src/bower-lib/angular-route/angular-route',
                       // 'angularScenario': 'src/main/resources/lib/angular-1.2.23/angular-scenario',
                        'pascal': 'src/bower-lib/angular-translate',
                        'translateStaticFilesLoader':'src/bower-lib/angular-translate-loader-static-files/angular-translate-loader-static-files'
                    },
                    shim: {
                        'angular': {
                            'exports': 'angular'
                        },
                        'jQuery': {
                            'exports': '$'
                        },
                        'nanoScroller': {
                            deps: ['jQuery']
                        },

                        'angular-ui.utils': {
                            deps: ['angular']
                        },
                        'angularMocks': {
                            deps: ['angular'],
                            'exports': 'angular.mock'
                        },
                        'angularResource': {
                            deps: ['angular'],
                            'exports': 'angular.resource'
                        },
                        'angularRoute': {
                            deps: ['angular'],
                            'exports': 'angular.router'
                        },
                        //'angularScenario': {
                        //    deps: ['angular'],
                        //    'exports': 'angular.scenario'
                        //},
                        'angularTranslate':{
                            deps: ['angular'],
                            'exports':'pascalprecht.translate'
                        },
                        'angularCookies':{
                            deps: ['angular'],
                            'exports':'ngCookies'
                        },
                        'translateStaticFilesLoader':
                        {
                            deps:['angular'],
                            'exports':'translateStaticFilesLoader'
                        }

                    }
                }
            }
        }

    },

// ALSO just loading files directly in karma fails. // // // //

module.exports = function(config) {

var conf = {
    basePath: '',
    singleRun: true,
    files : [
        // we can't do them all because it breaks
        {
            pattern: 'src/bower-lib/jquery/jquery.js',
            included: true
        },
        {
            pattern: 'src/main/resources/lib/nanoScroller.js',
            included: true
        },
        {
            pattern: 'src/bower-lib/angular/angular.js',
            included: true
        }, 
        {
            pattern: 'src/bower-lib/angular-mocks/angular-mocks.js',
            included: true
        }, 
        {
            pattern: 'src/bower-lib/angular-resource/angular-resource.js',
            included: true
        }, 
        {
            pattern: 'src/bower-lib/angular-route/angular-route.js',
            included: true
        },
        {
            pattern: 'src/bower-lib/angular-cookies/angular-cookies.js',
            included: true
        },
        {
            pattern: 'src/main/resources/lib/Stacktrace.js',
            included: true
        }, 
        {
            pattern: 'src/main/resources/lib/angular-translate.js',
            included: true
        },
        {
            pattern:'src/bower-lib/angular-translate-loader-static-files/angular-translate-loader-static-files.js',
            included: true
        },
        //bake down the templates
        {
            pattern: 'src/main/**/*.html',
            included: false
        },
        {
            pattern: 'src/main/**/*.gif',
            included: false
        },
        {
            pattern: 'src/main/**/*.Int.js',
            included: false
        },            
        {
            pattern: 'src/main/app/**/*.js',
            included: true
        }, 
        {
            pattern: 'src/main/commons/**/*.js',
            included: true
        },       
        {
            pattern: 'src/main/resources/lib/i18n/angular-locale_en-us.js',
            included: false
        },
        {
            pattern: 'test/KarmaSpecRunner.js',
            included: true
        },
        {
            pattern: 'src/templates.js',
            included: true
        }
    ],
    exclude: [
        'lib/angular-1.2.16/docs/**/*.*',
        'lib/angular-1.2.16/**/*min.js',
    ],
    plugins: [
        'karma-jasmine',
        'karma-phantomjs-launcher',
        'karma-requirejs',
        'karma-chrome-launcher',
        'karma-js-coverage'
    ],
    frameworks: [
        'jasmine',
        'requirejs'
    ],
    browsers: [
        'PhantomJS'
    ],
    preprocessors: {
        'src/main/app/**/!(*Spec|RangeSlider|DebuggerController*)+(.js)': ['coverage'],
        'src/main/commons/**/!(*Spec|SchemaDefinitions|Scrollable|Debug|DynamicLocale*)+(.js)': ['coverage'],
    },
    coverageReporter: {
        type: 'lcov',
        dir: 'test/coverage/'
    },
    reporters: [
        'dots',
        'coverage'
    ]
};

config.set(conf);

};

knalli commented 9 years ago

Mismatched anonymous defines.

What does this mean?

I'm sorry, but an UMD header is not uncommon and breaks not an existing environment. Well, unless you have indeed a AMD/CJS look-a-like env (i.e. having a define function in gloal scope) being not AMD. But that should not be the case.

I cannot extract a piece of qualified information of your ocnfigs, because I cannot look into it. However, you are in charge. If you are really sure the UMD header is the issue, you must be able trace the call of define in a debugger. And then: Which component provides it? Perhaps you have such a module system enabled?

Again: If this is a common problem, you would not be the first one reporting it and all demonstration would fail (plnkr/jsfiddle are mostly w/o an AMD system installed)

nthibert commented 9 years ago

I have just updated to 2.7.2 and I use requirejs for a lot of my projects. Everything with the changes made work well. Although, when I compile the code down and mix in to 1 file I get an exception "angular is not defined". I believe the issue comes from these lines:

define([], function () { return (factory()); });

I believe angular needs to be passed in as a dependency, then passed in to the factory as a param. IE:

define(["angular"], function (angular) { return (factory(angular)); });

I believe until this is fixed, I will have to revert to an older version.

knalli commented 9 years ago

@nthibert If you are really using RequireJs, you will probably know the paths config option.

We could require angular, but unless they support this directly, the current thing is more convenient.

vance commented 9 years ago

Where are these jsfiddles or plnkrs that use Jasmine + angular translate? The ones on the demos page do not include jasmine. https://github.com/angular-translate/angular-translate/wiki/Demos.

Like I said, this is a failure when trying to use with Jasmine/Require. If there was an example that would be great. Maybe I'm totally off base, but I don't have a good working scenario to look at. thanks for you time!

Just got to reading this: http://dontkry.com/posts/code/browserify-and-the-universal-module-definition.html

So far, in configuring our setup, we tried every variation of jasmine/karma configs to not include, do include, only load but not add to 'exports' in the config etc. which is why I lean toward it not being the setup, since only (and all) UMD style headers seem to break running under Jasmine. ng-table did this too. My suspicion is that if other people are having this problem, they're just deleting the UMD header, too.

knalli commented 9 years ago

I'm very sorry in case someone has problems with that.. but, really really, there is no fail (at least for us).

Again: People in the world using this for years, not only in this library but also in many other libraries, without any complaints. As you repeatedly pointing the finger at us, I will make it straightforward: The problem is on your side.

Regarding examples: Well obviously, we have no Jasmine tests on JsFiddle/Plnkr but regular demonstrations of code/usage scenrios examples at angular-translate.github.io as well as as several OTR demos: see here, here, here, here or here. These are my latest ones, that's sufficient.

Regarding Jasmine tests: Well, the easiest way is looking at our tests actually. We do not use RequireJS or any other module system, and, guess what, there is no problem.

(Please do not being confused with the scope's stuff, that is a little hack for testing against different types of AngularJS releases. More or less it's only a path-to-library resolver.)


One personal addition:

I don't know what you are thinking what magic a module loader is doing. At the end of the day, it's only a set of functions which executes code, reads dependencies specs and injects <script> tags (in case of a browser environment). No magic, just JavaScript code. Said this, if you have issues with loading such standard libraries, you must have installed and enabled such a module loader. Or you have defined the expected objects/functions. You can have a look at the UMD header: It is a simple and clean 3-way-branches-condition. I cannot debug your code, but you have access and can looks what happens there.

A module loader like RequireJS is great for bigger projects (at least in times before ES6+ modules). Despite that I highly recommend to understand what it is doing for you, what it can do for you.. and what it cannot do for you.

knalli commented 9 years ago

And here we go, a JSFiddle with Jasmine tests. It uses not the latest ones, that's a fast and messy prototype.

vance commented 9 years ago

Thanks for the example! I have configured (in my case) grunt-jasmine and karma-requirejs properly. There was a problem with the requirejs shim for Karma. Jasmine was trying to both load it from plain js resource and load it as a module, I think.

nthibert commented 9 years ago

@knalli Thanks for all the responses. I do have to disagree with your statement that the problem is on our side (there are some problems but I fix those ;) ). I may have not explained my situation properly. As I do know the paths property for requirejs. We implement this lib around the 2.5.x mark. While the implementation went smooth because we used a shim. Also, I like to tell you that we are using r.js to compile everything down into 1 file. The other day, I change the version to 2.7.2. This is when the error of "angular is not defined" appeared. So when I went looking, I found that when you used the UMD and then I compiled everything into 1 file (where angular is getting compiled into the same file) angular was not part of the closure or the global space. So everything started failing. So after your first message, I tried my approach and referred to another angular lib that is doing the same sort of thing (ng-table) and it worked. I think when it comes to using the UMD with AMD to not assume angular is loaded in the global space. Below is the implementation I spoke of.

(function (root, angular, factory) {
    if (typeof define === 'function' && define.amd) {
        // AMD. Register as an anonymous module unless amdModuleId is set
        define(["angular"], function (angular) {
            return (factory(angular));
        });
    } else if (typeof exports === 'object') {
        // Node. Does not work with strict CommonJS, but
        // only CommonJS-like environments that support module.exports,
        // like Node.
        module.exports = factory(angular);
    } else {
        factory(angular);
    }
}(this, window.angular || null,  function () {})

Please let me know if there is another way around this. Where I need to compile angular and angular-translate into the same file.

Your code is rock solid and easy to use. Please don't consider this as a personal issue. We are peers just reviewing code and making suggestions.

Thanks for the hard work.

knalli commented 9 years ago

Please don't consider this as a personal issue.

No worries.

Also, I like to tell you that we are using r.js to compile everything down into 1 file.

I'm doing this as well. Unfortunately, several libraries are not compatible (and not declaring its dependencies) which results into a config with paths. Obviously, we don't declare angular also. The reason is that I'm not sure what we should do. Does the name angular is globally the name for AngularJS? Or angular.js? Or angularjs? At the moment everyone can use its own name.

I'm not happy with the current one, but on the other side I do not want to force someone to name its dependencies. Thoughts?

Edit: We have added messageformat.js few days ago (#1068).

vance commented 9 years ago

Turns out I had used 2.5.2 instead of 2.7.2. When I said it worked, Bower was always using the old version due to another dependency by translate-loader. After manually copying the UMD version (2.7.2) over, we get the error that the module cannot be found at all. So, what is the reasoning behind using UMD? How could this be resovled in Karma?

This is the current Karma runner requirejs config:

requirejs.config({

    baseUrl: '',
    paths: {
        // External libraries
        'angular': '/base/src/bower-lib/angular/angular',
        'jQuery':'/base/src/bower-lib/jquery/jquery',
        'angularMocks': '/base/src/bower-lib/angular-mocks/angular-mocks',
        'angularCookies': '/base/src/bower-lib/angular-cookies/angular-cookies',
        'frisby': '/base/test/lib/frisby',
        'pascalprecht.translate': '/base/bower-lib/angular-translate/angular-translate',
        'angucomplete-alt': '/base/bower-lib/angucomplete-alt/angucomplete-alt' // THIS LIB HAS SAME PROBLEM
    },
    shim: {

        'frisby': {'exports': 'frisby'},

        'angular': {'exports': 'angular'},
        'angular-ui.utils': {deps: ['angular']},
        'angularMocks': {deps: ['angular'], 'exports': 'angular.mock'},
        'angularCookies': {deps: ['angular'], 'exports': 'ngCookies'},
        'angucomplete-alt': {deps: ['angular'], 'exports': 'angucomplete-alt'},
        'pascalprecht.translate': {deps: ['angular'], 'exports': 'pascalprecht.translate'},
        'jQuery': { 'exports':'$'}    ,

    },

    // Ask Require.js to load these files (all our tests).
    deps: dependencies,
    // Set test to start run once Require.js is done.
    callback:window.__karma__.start
});
vance commented 9 years ago

*addendum: seems to work fine in Jasmine, for some reason only the Karma runner has this issue.

knalli commented 9 years ago

@vance We have already pointed out the issue is not UMD nor angular-translate. I'm sorry, but I'll close this issue now. If you have general issues understanding UMD, AMD or any other module concepts, I'll recommend stackoverflow with a much greater use base.

Thank you in advance.

vance commented 9 years ago

That's usually my first resource, and I've exhausted it. I've got a few thousand points there, i'm no stranger =)Thanks for looking at this. I guess I can chalk this up to having some dependency error and maybe need to update Karma and RequireJS, though its only a minor dot version upgrade. Seeing as all my other dependencies work fine, i'm going to downgrade to translate 2.5.

vance commented 9 years ago

http://i.imgur.com/bKGj4no.jpg?1

Stefan-Rein commented 8 years ago

First things first: Without the r.js optimizer and trying to include all modules in one file, everything works fine. Assuming you use the r.js optimizer and the mainConfigFile for configuration, you maybe want some setup likes this:

You want to use one module name (AMD module id) and get all dependencies in the right order. (for the „name“ property in the options in r.js) Lets call this „appStarter“, by which the optimizer should put all modules in the right order (because of putting all modules in one big file). Non AMD dependencies should be defined in the shim of the require config file and your app dependencies from the main app module.

Like this:

require.config({
...
    paths: {
        appStarter: "./app"
    },
    shim: {
        appStarter: {
            deps: ["angular"]
        }
    }
...
});

Setting this up we get the non AMD dependencies first in the file and then the rest modules defined in the app.js in the right order.

But now, if you want to load, say some service for the translate-module e.g. „storage-local“, you need to define the dependency on the angular-translate module, so it is loaded after the translate module. But now the translate module has to be defined in the shim.

And the output, from before:

define([], function () {
    return (factory());
});

becomes:

define('angularTranslate',[], function () {
    return (factory());
});

And now, instead of being directly invoked from the IIFE, the module just get assigned and the factory function isn’t called, therefore getting this message: Module 'pascalprecht.translate' is not available!

angular-translate.js Fix 1: Call factory() after the define (angular-translate.js) and the angular module is defined.

Fix 2: Remove the UMD header

r.js:

...
name: "appStarter",
insertRequire: ["appStarter"],
out: "app/build.js",
skipModuleInsertion: true,
...

require.config

{
    paths: {
        "appStarter": "./app",
        "angular": "../path/to/angular",

        "angularTranslate": "../path/to/angularTranslate/angular-translate",
        "angularTranslateLoaderPartial": "../path/to/angularTranslate/service/loader-partial",

        "angularTranslateCookie": "../path/to/angularTranslate/service/storage-cookie",
        "angularTranslateLocal": "../path/to/angularTranslate/service/storage-local",
        "angularCookies": "../path/to/angular/angular-cookies.min"
    },
    shim: {
        "appStarter": {
            deps: [
                "angular",
                "angularCookies",

                "angularTranslate",
                "angularTranslateCookie",
                "angularTranslateLocal",
                "angularTranslateLoaderPartial"
            ]
        },
        "angularCookies": {
            deps: ["angular"]
        },

        "angularTranslate": {
            deps: ["angular"]
        },
        "angularTranslateCookie": {
            deps: ["angularTranslate"]
        },
        "angularTranslateLocal": {
            deps: ["angularTranslate"]
        },
        "angularTranslateLoaderPartial": {
            deps: ["angularTranslate"]
        }
    }
}
...

Any better ideas? Thanks in advance!

knalli commented 8 years ago

@Stefan-Rein: I don't understand both "fix solutions", but the RequireJS configuration looks fine; you don't have to shim your own app (however somehow it would make sense). Shimming the libraries as business as usual.

@Stefan-Rein @vance I'm still not getting the problem. Can we please make up an isolated prototype (small as possible) for this? Eventually the best case would be (adhoc) repository here at github.

Stefan-Rein commented 8 years ago

Hey, thanks for the fast answer!

I try to describe the problem: With the above described setup and the original angular-translate module (v.2.7.2) my SPA crashes with following error: Module 'pascalprecht.translate' is not available!

With the "fix solutions" the occurring error doesn't come up, everything work as expected.

I'm setting something up.

Stefan-Rein commented 8 years ago

Here is a repository:

https://github.com/Stefan-Rein/angular-translate-requirejs-problem

Please do the following: index.html (now working, default loading with require):

Comment current script tag out and use the build.js script tag You get an error in the console, app crashes.

Now go in: assets/libs/angularTranslate/angular-translate.js And add outcommented the line 15

knalli commented 8 years ago

@Stefan-Rein Okay, two points.

At first: Perhaps that is only an issue in this demo, but... why do you inject source files from angular-translate instead of using bower or npm? At least files in sub directory service/ are not part of any distribution.

Secondly: I've revisited the output (build.js). Your main entrypoint does not have any dependencies in the target artifact

define('appStarter',[
        "./controller/AppController",
    ],
    function (
        AppController
    ) {

Unless you define global dependencies on startup, each module should declare its own dependencies (even angular).

Basic solution: Define at least angularTranslate

define('appStarter',[
        "./controller/AppController",
        'angularTranslate'
    ],
    function (
        AppController
    ) {

OT: You're from Hannover, too?

No, not me. I'm from Cologne. Perhaps you mean Pascal ;)

Stefan-Rein commented 8 years ago

@knalli I just copied some files around for example - but yes, I would use npm and include the plugins.

Your solution is working. My fault was to think it should be enough to declare the angularTranslate dependency just in the shim of the config.js for the appStarter. But of course since your plugins are AMD, I need to call them.

Thank you! Maybe the ones above had the same problem and it could save someone some time with the same issue.