Kendo-ui-core: Modules should define/require with relative paths not absolute

Created on 18 Aug 2016 · 14Comments · Source: telerik/kendo-ui-core

Bug report/Feature request

Bug report

Reproduction of the problem

project structure

- /
  - bower.json
  - package.json
  - /src
    - index.html
    - /css
    - /app
      - myapp.js
    - /lib
      - /jquery
        - /dist
          - jquery.js
      - /kendo-ui
        - /js
          - kendo.core.min.js
          - kendo.datepicker.min.js
          - ...
      - /dojo
          - dojo.js

.bowerrc

{
  "directory" : "src/js/lib"
}

bower.json

  "dependencies": {
    "kendo-ui": "kendo-ui-core#^2016.2.812",
    "jquery": "^2.1.1",
    "jquery-ui": "^1.12.0",
    "dojo-util": "^1.11.2",
    "knockout": "^3.4.0",
    "knockout-kendo": "^0.9.7"
  },

index.html

 <html lang="en">   
    <head>   
        <meta charset="UTF-8">  
        <title>Title</title>  
    </head>  
    <script>
       dojoConfig = {          
            baseUrl: "js",
            async: true,
            tlmSiblingOfDojo: false,
            parseOnLoad: false,
            packages: [
                {name: "dojo", location: "lib/dojo"},
                {name: "dojox", location: "lib/dojox"},
                {name: "dijit", location: "lib/dijit"},
                {name: "dojox", location: "lib/dojox"},
                {name: "jquery-amd", location: "lib/jquery/dist"},
                {name: "kendo-ui", location: "lib/kendo-ui/js"},
            ],
            deps: [
                "kendo-ui/kendo.core.min",
                "kendo-ui/kendo.datepicker.min"
            ],
            map: {
                "*": {
                    "$": "jquery-amd/jquery",
                    "jquery": "jquery-amd/jquery"
                }
            }
        };
    </script>
    <script src="js/lib/dojo/dojo.js"></script>
    <body>
        <input id="dpicker" type="datetime" />
        <script>
            require(["dojo/ready", "$"], function(ready, $){
                ready(function(){
                    $('#dpicker').kendoDatePicker();
                });
            });
        </script>
    </body>
</html>

Current behavior

All Kendo UI modules use the absolute module names for each module.

For example:

define("kendo.datepicker.min",["kendo.calendar.min","kendo.popup.min"],e)

This tries to load the module from `js/kendo.popup,min' which does not exist. If the code was written as such:

define("kendo.datepicker.min",["./kendo.calendar.min","./kendo.popup.min"],e)

it would not fail and Kendo UI would work with both the Dojo AMD loader and the Require JS loaded no matter where the Javascript code is located.

Expected/desired behavior

Modules from the same package should always use ./ style loading when using AMD loaders. This prevents the situation where a AMD module such as Kendo UI can exist anywhere in the project structure instead of having to exist in baseUrl.

Environment

Kendo UI version: 2016.2.812
jQuery version: 2.1.1
Browser: [all]
AMD Loader: [Dojo 1.10+]

Source

andrewfinnell

Most helpful comment

Well, I am certainly glad that we figured this out. Now, let me try explaining the problem with the named AMD modules. It's a long and somewhat amusing (if a bit sad) story. Notice that I will refer to files which are not present in this repository, because Kendo UI Core is a subset of Kendo UI Professional.

When Kendo UI started, we did not use any modules in our source code at all. kendo.all.js was produced by simple concatenation. Internally, we used multiple files like kendo.core.js, kendo.dropdownlist.js.

Next in our roadmap are the dataviz widgets and the HTML editor. It is impractical to develop those in a single file, so internally, we separate their code into multiple files in the chart and editor directories. However, modules are still not that common, so we decide to concatenate and ship them as kendo.editor.js and kendo.chart.js, to facilitate the scenario of people including the files manually in their head tag.

At this point, our distribution contains:

kendo.core.js and several other files, which are basically the same as the originals:
kendo.editor.js and the other big components which are like bundles, but not exactly, as they bundle only their widget specific files, still depending on other files.
kendo.all.js which is a "true" bundle, depending on jQuery.

Things start to get messy from this point on.

Next thing, AMD modules became mainstream and people start requesting support for them. No problem - we wrapp each of the files in AMD. So far so good. We still rely on homegrown concatenation and dependency resolution.

Next thing, our code base becomes more and more complex, as widgets like the scheduler and the gantt appear. Our homegrown dependency resolution becomes more complex and hard to maintain. We hit several limitations, related to cultures and messages.

We decide to remove it in favor of placing AMD wrappers in our original source code and using one of the many AMD tools out there. Full disclosure, I am the one guilty of that. However, we are still bound to the distribution format outlined above, including the semi bundle files for the editor and the chart.

It turns out that we can still support the semi-bundles by using the so called named modules. In a nutshell, they allow you to have multiple AMD modules in a single file. The catch - the file must be required with the exact name of the module you wish to execute. We apply the named modules bundling to all output files.

We release this, breaking everyone that used the previous AMD format. Sorry, everyone :(. We had to do that, eventually. After a while, several workarounds emerge, and we document them. We do our best to help everyone, and we acknowledge the problem.

Around this moment, we also start shipping Bower. Since it was not entirely clear how bower packages we supposed to be consumed, so we just shipped the distribution format that we already got.

Fortunately, after a while, NPM won over Bower. In it, we see a chance to fix our case, because we are not bound to the current distribution format. Since the NPM contents are somewhat opaque to the end user, we just drop the semi-bundles format and ship the files as-is, using Webpack to change their module format from AMD to CJS. We start recommending using NPM over Bower and as a format for people who want to require individual files in general. People pick it up, and report that it works for them. Partial redemption is achieved. And this is how I end this story of a JS library evolving throughout the years.

Back to you @afinnell . I appreciate your interest and desire to help, but I am afraid that it involves parts of our codebase (and our distribution model) which we have not open-sourced, and I don't think that you will be able to figure something out without access to the entire repo. To be honest, I don't think that it is possible at all, or worth it. NPM is a great format, distribution model and a vibrant ecosystem, which we highly recommend.

petyosi on 18 Aug 2016

👍2

All 14 comments

I am afraid that the named AMD modules approach is necessary due to some files containing multiple modules. We are not aware of any means to overcome that, because of the files structure which we should maintain for compatibility reasons. As an alternative, I may suggest that you look into using the NPM module distribution channel.

petyosi on 18 Aug 2016

😕1

I'm not sure my description of the bug is being understood. This doesn't have to do with NPM, is had to do with properly referencing sibling modules when using AMD. All sibling modules should be loaded using the relative URL not an absolute. Using the relative URL would not break any existing customers.

The AMD loading is incorrect from what I can tell. Here is a Stackoverflow explaining the situation:

http://stackoverflow.com/questions/14548369/requirejs-relative-paths

And AMD documentation on using relative pathing in the same package:

https://dojotoolkit.org/documentation/tutorials/1.10/modules_advanced/#writing-portable-modules

and here is the AMD spec on the issue:

https://github.com/amdjs/amdjs-api/wiki/AMD#user-content-define-id-notes

This doesn't have anything to do with NPM or how the Kendo UI package is imported into a project. It has to do with the fact that the code, from what I can tell, uses absolute module names, thus breaking the AMD spec. I can submit a partial patch to demonstrate the fix.

This should not be a closed ticket.

andrewfinnell on 18 Aug 2016

Looking at the code in Git it appears it uses the relative pathing:

https://github.com/telerik/kendo-ui-core/blob/master/src/kendo.list.js

(function(f, define){
    define([ "./kendo.data", "./kendo.popup" ], f);
})(function(){

here is what it looks like minified from NPM repo when pulled down with _Bower_:

define("kendo.list.min",["kendo.data.min","kendo.popup.min"],e)

The minified version that is published to NPM does not keep the relative pathing in place. From my naive perspective it seems to be a problem with the published package then. However, if it has to do with how I'm pulling it down with _Bower_ I wouldn't mind a description on how to properly pull it down.

andrewfinnell on 18 Aug 2016

The format you see is not a problem - it is an intentional design decision, which is implemented as a build step, due to the reasons I listed in my previous reply.

The NPM registry does not contain amd modules - check this link for its contents:

https://npmcdn.com/[email protected]/js/kendo.list.js

petyosi on 18 Aug 2016

The build works correctly as it produces relative module paths. The Minified version which is pulled down with npm install kendo-ui-core or bower install kendo-ui-core removes the relative pathing as it assumes it is going to be minified with the rest of the application, thus working.

The real answer to this is a published non-minified version of kendo-ui-core as a NPM package. I can see how kendo-ui-core would work IF all of the files are minified together with the rest of the application. However, this is not reasonable during the development phase for obvious reasons. It prevents using kendo-ui-core with a package management tool until the full product is ready to be shipped (i.e. built together into a single minified .js file)

Here is a very simple Use Case:

mkdir kendo-ui-test
cd kendo-ui-test
bower init
bower install kendo-ui-core --save
cat bower_modules/kendo-ui/js/kendo.autocomplete.min

You'll see

!function(e,define){define("kendo.autocomplete.min",["kendo.list.min","kendo.mobile.scroller.min"],e)}

The minified version is assuming it will never be used in a non-production build. The published package of kendo-ui-core should include both debug and minified versions. As it stands, the published kendo-ui-core package is basically useless except as a final build step for a finished product. Thus, it doesn't make any sense to EVER use it, as you can get the non-minified version for development purposes, then minify everything together oneself, thus not needing the published kendo-ui-core package.

I do not see how any developer can use the NPM kendo-ui-core package as it stands. This has to do with the published package, not Kendo UI itself. Kendo UI itself is fine. I am referencing the published NPM package only.

Here is a Use Case of a different NPM package that works:

mkdir dojo-test
cd dojo-test
bower init
bower install dojo-util --save
cat bower_modules/dojo/back.js

You'll see

define(["./_base/config", "./_base/lang", "./sniff", "./dom", "./dom-construct", "./_base/window", "require"],

I completely understand you have a build process that converts the Kendo UI core into a AMD compatible file. That conversion _works_ and does not have a problem. The problem lies in the published NPM package as it ONLY contains a minified version which is not compatible with anything but a final production build.

If you run my Test Case you'll easily see the problem. The NPM package for kendo-ui-core will not work with RequireJS nor Dojo AMD as it has currently been published without a specific module mapping.

I have created a demonstration of all of this here: https://github.com/afinnell/kendo-dojo-app

It doesn't matter if kendo-ui is put in src/js/lib or bower_modules. The result is the same. The ONLY way the NPM package will work is if it gets minified with everything else in the app, thus making it impossible to debug anything, not just kendo.

andrewfinnell on 18 Aug 2016

I started to think that I was going crazy as I did see that the NPM package does include the non-minified version, however, it also uses absolute pathing. So why does the code in github have the correct relative pathing and the NPM package which checkout's from github convert it to absolute pathing? I believe it has to do with this file: https://github.com/telerik/kendo-ui-core/blob/master/build/gulp/gather-amd.js

https://github.com/telerik/kendo-ui-core/blob/master/src/kendo.button.js

(function(f, define){
    define([ "./kendo.core" ], f);
})(function(){

Incorrect Module Loading
bower install kendo-ui-core
bower_modules\kendo-ui\src\js\kendo.button.js

(function (f, define) {
    define('kendo.button', ['kendo.core'], f);
}(function () {

Correct Module Loading (With patch to kendo build tools)

(function (f, define) {
    define('kendo.button', ['./kendo.core'], f);
}(function () {

andrewfinnell on 18 Aug 2016

To be honest, I am getting lost in the comments above, and I am the developer who implemented both package mechanisms. So I will go ahead with I know.

1) The bower distribution, and the AMD implementation in general is very flawed, but we can't change it for historical reasons. I don't like the fact that one should use baseUrl. There are plenty of discussions of this case in the issues of this repository and in our forums.

2) The NPM package contents are correct, in my book. They are consumed with minimal to no additional configuration by popular tools like browserify and webpack - check this repository for some examples.

https://github.com/telerik/kendo-ui-npm-example

petyosi on 18 Aug 2016

Those examples work because they rely on a build tool to fix the AMD module id's. Which is what I described in the comments above. It will work if a build tool _fixes_ the module path's. Out of all the packages I've 'installed' with bower, kendo-ui is the only one with absolute path's in the define function when referencing their own internal modules. Referencing "jquery" is correct. Referencing "kendo.core" is not correct. It should be "./kendo.core". The AMD spec talks about this in the link I referenced.

At this time the kendo-ui package is relying on a third-party build tool to fix the module dependencies in the define function.

I understand what you're saying, however, it is objectively incorrect for a package to load it's own modules using absolute module id's. The kendo.button module should reference the kendo.core module using './kendo.core' not 'kendo.core'. The examples only work due to an additional build step fixing the module id's.

I have a work around so at this time it is not a show stopper. However, without specifically mapping ALL of the kendo module's to their correct locations, or using a third-party build tool to fix the module id's, the package isn't usable.

Perhaps this is a Use Case that is moot as it's the norm to use something like browserfy in all cases, thus, removing the issue. I will update my example to use browserfy to see if that fixes the issue.

However, if you're able to take any time to look at ANY other NPM package which has more than 1 AMD module, you'll see that they self-reference their own modules using relative paths.

andrewfinnell on 18 Aug 2016

NPM packages use cjs module format, so I am not sure that I understand your what you mean here. At any case, you can check the link I gave:

https://npmcdn.com/[email protected]/js/kendo.list.js

search for the kendo.core.js - it is referenced with a relative path.

petyosi on 18 Aug 2016

Yes, however, the scripts that are ran when you install the module removes the relative path. If you run bower install kendo-ui-core on any machine you'll see that all the relative paths are removed after the module is installed. If this is only on my two machines then perhaps it's my environment.

However, the resulting .js files from the installed packaged do NOT have relative paths as they are removed by the amd optimizer code in the kendo package.

If you're willing to indulge me, please post the define statement in the installed kendo-ui-core package after you install it with bower. You'll see that it's been changed.

andrewfinnell on 18 Aug 2016

Let's start from the fact that bower and npm are not the same thing, and they use different sources. To avoid repeating myself, please check this comment:

https://github.com/telerik/kendo-ui-core/issues/2080#issuecomment-240734943

petyosi on 18 Aug 2016

👍1

I misread that comment and you are correct in that the NPM one is correct. With that said, can you point me to the discussions about the Bower distribution? Perhaps that would be a better place for me to move my thoughts and let you get back to actual bugs with kendo itself.

I appreciate the patience and see what you're saying about the NPM distribution versus the Bower distribution. Thank you for sticking in there and making sure I saw that. Kudos to you. I mean it.

I'd like to work with the Bower forum to determine why the distribution requires absolute paths for historical reasons if that's acceptable. Regardless, you have provided a mechanism to use Bower and kendo with browserfy so I'll let this beast of a Issue rest in peace.

My thanks and may the rest of your week not be as difficult as this single Github Issue. :)

I'll search for the Bower forums for the discussion you reference.

andrewfinnell on 18 Aug 2016

At this point, our distribution contains:

kendo.core.js and several other files, which are basically the same as the originals:
kendo.editor.js and the other big components which are like bundles, but not exactly, as they bundle only their widget specific files, still depending on other files.
kendo.all.js which is a "true" bundle, depending on jQuery.

Things start to get messy from this point on.

Around this moment, we also start shipping Bower. Since it was not entirely clear how bower packages we supposed to be consumed, so we just shipped the distribution format that we already got.

petyosi on 18 Aug 2016

👍2

@underlog I appreciate the backstory greatly. I had some time off and just now saw this. Everything makes perfect sense and I can empathize with the situation.

Thank you again.

andrewfinnell on 26 Aug 2016

Was this page helpful?

0 / 5 - 0 ratings

Related issues

CSP

aersamkull · 4Comments

When importing an xlsx with hidden columns to Spreadsheet, the hidden columns letter headings (A, B, C etc.) remain visible and overlap each other

veselints · 5Comments

Unordered List(ul) are not correctly exported to PDF

simonssspirit · 3Comments

Items cannot be expanded/collapsed in Grid with server hierarchy

VicTachev · 5Comments

Media player is not rendered correctly in Mobile

VicTachev · 5Comments