qooxdoo / qooxdoo

qooxdoo - Universal JavaScript Framework
http://qooxdoo.org
Other
764 stars 260 forks source link

Manifest.json and add-script/add-css. #9129

Closed jochumdev closed 3 years ago

jochumdev commented 7 years ago

I've written grunt-qx, when you want to add a library which needs external css/js you need to define these in the options of it:

{
      addScript: [
        '%(QXPROMISEREST)s/resource/qxpromiserest/js/fetch.min.js',
        '%(QXPROMISE)s/resource/qxpromise/js/bluebird.min.js',
        '%(QXPROMISE)s/resource/qxpromise/js/alameda.min.js'
      ]
}

Here i want do discuss a way to define these in Manifest.json so grunt-qxcompiler can pull it from there and include these if the the user wants that.

What do you think about that in Manifest.json library called qxc.promise:

{
  "info": {
    "name": "qxc.promise",
  },
  "dependencies": {
    "qooxdoo-sdk"
  }
  "externalResources": {
    "script": [
      "resource/qxc/promise/js/bluebird.min.js",
      "resource/qxc/promise/js/alameda.min.js"
    ]
  }
}

And for the for imaginary library qxc.promiserest:

{
  "info": {
    "name": "qxc.promiserest",
  },
  "dependencies": {
    "qooxdoo-sdk",
    "qxc.promise"
  },
  "externalResources": {
    "script": [
      "resource/qxc/promiserest/js/fetch.min.js"
    ],
    "css": [
      "resource/qxc/promiserest/css/style.min.css"
    ]
  }
}

And finaly the app drawstack:

{
  "info": {
    "name": "drawstack",
  },
  "dependencies": {
    "qooxdoo-sdk",
    "qxc.promiserest"
  }
}

The user can then use any package manager or a VCS (like git submodules) to get his packages.

grunt-qxcompiler will have an option to define library dirs where it will search for directories with a Manifest.json and include them if required.

I hope we can figure a nice standard for these out.

level420 commented 7 years ago

for this I'm currently using the asset hint in the class file where I'm using the external stuff like:

/**
 * @asset(myapp/qxpromise/*)
 */

and generator.py does the magic for me. Don't know yet how this works in qxcompiler.

I suspect that this only works if the files need to be loadad after the qx loader.

level420 commented 7 years ago

btw. do you really mean Manifest.json, not config.json?

jochumdev commented 7 years ago

@level420 Thanks for looking into it.

Have to explain the root cause a bit more:

My goal:

{
   qxcompiler: {
      options: {
        libraryDirs: [
         qxpath + '/framework',
         './node_modules/qxc.promise',
         '.'
       ]
     }
  }
}

This would do the add-script and add-css tasks as documented here for config.json with grunt-qx.

If you look at the README of grunt-qx you will see a lot of addScript lines, i want them to be in the Manifest.json.

level420 commented 7 years ago

Then maybe I'm the wrong person to discuss this, as I've not yet used qxcompiler

jochumdev commented 7 years ago

The thing is not qxcompiler, the thing is howto add add-css and add-script to Manifest.json, qxcompiler doesn't know about config.json as well as ./generate.py with libraries.

johnspackman commented 7 years ago

So my first thought was that's fine because we would just propose a change to the Manifest.json schema; but how would we know to use those directives? If they were in Manifest.json then any reference to any class in that library would trigger their inclusion, but this would not be right if (for example) you had a utility library, and only one of your many widgets required the scripts.

The correct way to do this would be to add compiler hints in the class header comments, in much the same way that we use @asset to reference resources we would define @addScript and @addCss. The only problem with this would be that these hints are only used by the generator, and there would need to be a mechanism to pass this information through to qx.Class so that at runtime the JS/CSS can be included.

There is already a mechanism to pass compiler hints through to the runtime - Annotations.

So in a PR I'd start by defining an annotion (eg) qx.anno.resources.AddScript and qx.anno.resources.AddCss and then modify qx.Class to detect those annotations at class level to implement the references. The classes would look like this:

    qx.Class.define("mypackage.MyClass", {
        extend: qx.core.Object,
        "@": [ 
            new qx.anno.resources.AddScript([
                'mypackage/qxpromise/js/es5-sham.min.js',
                'mypackage/qxpromiserest/js/fetch.min.js',
                'mypackage/qxpromise/js/es5-shim.min.js',
                'mypackage/qxpromise/js/bluebird.min.js',
                'mypackage/qxpromise/js/alameda.min.js'
                ]
        ],

        members: { /* ... snip ... */ }
jochumdev commented 7 years ago

One point of the ./generate.py add-script/add-css is that it adds those scripts and css before Init, can this happen with annotations to?

jochumdev commented 7 years ago

@johnspackman and thanks for your response.

johnspackman commented 7 years ago

Yes, because the scripts are not necessarily needed before any qx classes are loaded. However, the boot loader will typically use async loading of scripts so the load order is not really guaranteed (technically speaking anyway) - this would imply that the classes that depend on these scripts are required to detect whether the scripts are loaded yet, although I bet that in 99% of cases it just works out OK.

If the scripts were loaded when the class was created by qx.Class.define, then any timing vulnerabilities would have to be detected and dealt with (assuming that forcing synchronous loading of scripts is unacceptable) because they are more likely - eg constructors would not be able to assume the scripts had finished loading.

We could implement a feature to "preload" the scripts, but this would mean blocking the app startup until the scripts have loaded - and there would have to be error control to deal with load failures anyway. I would suggest instead than on demand loading of external scripts is implemented, including events fired when they are available.

PS - once up on a time there was a ScriptLoader class in qx (around v3 I think) but for some reason it was removed.

@level420 those @asset directives - do they cause the CSS to be loaded? I thought that they would only cause them to be available as managed resources and you'd still have to load them manually (qxcompiler does not load external .css or .js automatically)

qx-bug-importer commented 7 years ago

loads css and js and works well :)

https://github.com/oetiker/QxJqPlot/blob/master/source/class/qxjqplot/MPlot.js

thron7 commented 7 years ago

I can also only speak from the Generator side of the view.

One thing is to tell the Generator that you want to have some resources included - the "what". The other thing is the question at which point in time those resources should be loaded into the browser - the "when".

There are already means to specify the "what", the @asset hint being the canonical one. It only makes sure the specified resources are available under the resource/ path, but nothing more. For an arbitrary resource like a .js file, a common qooxdoo idiom would be to specify them in the main class with @asset and then e.g. load them with XHR in the construct member. I think that's the way Tobi created some qooxdoo wrappers for general JS libraries (see the link to QxJqPlot above; and actually, as it is a common idiom it could be implemented in the framework).

This is early enough for most situations where you can control the first call into the third-party code. I don't know of many situations where it is compulsory that the third-party code has to be loaded before the main library class, or even before qooxdoo's init.

So this is all done in class code, and works whether you are writing an application or a utility library, and I would think hard if this is not enough, or if you really need to extend Manifest.json.

Still, there might be information that cannot be sufficiently expressed in class code. One thing of that nature that has been in the back of my mind for a long time are compiler options (just think of a library that demands that a specific compiler optimization be on or off for its code). And if you get to the point where you say an external .js file needs to be loaded before any library or even the qooxdoo runtime, this would be another situation that requires that.

I see two natural ways to deal with that. One is extending Manifest.json directly, as you suggest. The other is providing a "library-level" config file. I call this library-level as it contains settings that are relevant if the code is used by another application as a library. The standard config.json would still be responsible for running jobs on the library as a stand-alone app. We did this a lot for the standard apps and components of the qooxdoo SDK (look e.g. at component/apiviewer/api.json).

It is basically saying that if you want to use a library you not only have to reference its Manifest.json in your config.json, you also have to include its library-level config file. Currently, the using applications have to do this explicitly (e.g. by including api.json, what every skeleton does via application.json). But you could make this an automatic include the Generator takes care of. Rather than extending Manifest.json with multiple new keys (potentially duplicating logic from config.json) you could just add a single new key "library-config" that holds the name of the library-level config file, and the Generator would include this in the using application. So with minimal effort you'd have all the possibilities of the config system connected to the Manifest.json.

(This approach would also immediately solve another pain point of the Generator: transitive dependencies.)

You could also achieve some of this with annotations, as John suggested. Though all annotations are processed at compile time, there is nothing keeping you from creating an annotation that generates a list of resource URLs for the early loading function in the qx loader, much like @asset generates resource URLs that are loaded together with their class code. But I would really recommend that the annotation system be minimal, and reserved for information that cannot be sensibly allocated elsewhere.

jochumdev commented 7 years ago

In my case i need to preload es5-shim, es6-shim, fetch, bluebird and requirejs/alameda.

The shims, fetch and bluebird realy need eager loading.

jochumdev commented 7 years ago

@thron7 so your proposal to solve this goal is an api.json in the library and include that instead of Manifest.json, right?

thron7 commented 7 years ago

Not instead of the Manifest.json, but in the Manifest.json have a key like

...
   "library-config" : "api.json"
...

Then this .json file would get included automatically in the overall configuration of the build when an application references the Manifest.json. And api.json could contain add-script, add-css and any of the other config features.

thron7 commented 7 years ago

BTW: If you concern yourself with a Grunt-based tool chain, have a look at the next repo.

jochumdev commented 7 years ago

Want to/Should i extend that next repo with grunt-qxcompiler?

thron7 commented 7 years ago

I'm not deep enough into Grunt, so cannot say if the Grunt structure chosen in Next is worth taking forward, or if somebody with Grunt knowledge would rather start from scratch.

But I think the general idea taken in Next is still worthwhile, to use a generic JS-based build system like Grunt and have it side-by-side with the Pyhton-based Generator for a transitional period of time. Users would invoke grunt for all their command-line tasks, but under the hood Grunt would dispatch some of the jobs to the old Generator, and do the others in JS if they are already implemented (This is what we already had in the main qooxdoo repo before it was forked out into Next).

This would allow for a smooth transition from Python to JS, without any pressure to get things done by a specific date. The big jump would be in the beginning for the users when you remove generate.py and tell them to use grunt source instead of generate.py source. Under the hood, the Python code could still do most of the work, and over the following time the functionality could be ported to Grunt/JS piecewise, as devels can manage. In the end, everything runs in Grunt/JS and you can remove the Python tool chain.

If this still makes sense for most people, I would recommend bringing the Grunt code back from Next to the main repo and integrating qxcompiler with it (not necessarily in that order, you could of course start in Next and integrate qxcompiler first).

zaucker commented 7 years ago

I guess

generate.py

could also be made a wrapper printing out a warning and then calling grunt for a transition period. And the just print the warning and exit in a later release.

Cheers, Fritz

On Sat, 27 Aug 2016, thron7 wrote:

I'm not deep enough into Grunt, so cannot say if the Grunt structure chosen in Next is worth taking forward, or if somebody with Grunt knowledge would rather start from scratch.

But I think the general idea taken in Next is still worthwhile, to use a generic JS-based build system like Grunt and have it side-by-side with the Pyhton-based Generator for a transitional period of time. Users would invoke grunt for all their command-line tasks, but under the hood Grunt would dispatch some of the jobs to the old Generator, and do the others in JS if they are already implemented (This is what we already had in the main qooxdoo repo before it was forked out into Next).

This would allow for a smooth transition from Python to JS, without any pressure to get things done by a specific date. The big jump would be in the beginning for the users when you remove generate.py and tell them to use grunt source instead of generate.py source. Under the hood, the Python code could still do most of the work, and over the following time the functionality could be ported to Grunt/JS piecewise, as devels can manage. In the end, everything runs in Grunt/JS and you can remove the Python tool chain.

If this still makes sense for most people, I would recommend bringing the Grunt code back from Next to the main repo and integrating qxcompiler with it (not necessarily in that order, you could of course start in Next and integrate qxcompiler first).

Oetiker+Partner AG tel: +41 62 775 9903 (direct) Fritz Zaucker +41 62 775 9900 (switch board) Aarweg 15 +41 79 675 0630 (mobile) CH-4600 Olten fax: +41 62 775 9905 Schweiz web: www.oetiker.ch

johnspackman commented 7 years ago

There's a few things being discussed here:

A library level config would be useful where the library wants to add jobs or functionality to the build process, but with the intention being to switch to a grunt based toolchain it would be great if we could find a way to avoid having to add features to generate.py. AIUI grunt would allow processes to be added for depended-on libraries out of the box, allowing the library to do whatever it needed.

But that's a separate issue - for the issue of adding scripts, Thomas @thron7 is right that actually it's easier to handle that by @asset to make the script available from resources and then manually loading it, because that way you have access to all of the onload and onerror events. IE if you have to detect whether it has been loaded, you have to connect to some kind of script-loader class anyway to ask if your files have been loaded, so why not use that opportunity to tell it which files you want loaded.

IMHO the scripts are a dependency of specific classes and not necessarily a dependency of an entire library unless as they must be loaded before qooxdoo; in which case, I agree with Thomas that they are very unusual and I have to wonder if it's worth catering for that special case.

@pcdummy are you loading the shims, fetch, bluebird, and alameda in order to allow you to load something else? Or is Bluebird the bit that you want to add? Im wondering why a Qooxdoo wrapper to Bluebird or some other class couldn't just handle the script loading.

thron7 commented 7 years ago

@zaucker Yes, sure, you could have it the other way round. I guess we found it better to have the 'big bang' upfront, so people could get used to NPM downloads happening when you invoke the tool chain, and would understand why this is the case ...

jochumdev commented 7 years ago

@johnspackman i need the es5-shim currently only for .bind(this) in IE11 when the original Promise is not a bluebird promise, the es6-shim only for String.prototype.startsWith() i could easily replace both but i thought it always good to have them. fetch (which sets window.fetch) i need for restful.js.

jochumdev commented 7 years ago

@johnspackman ahh and yes, finaly i load something else with alameda.

zaucker commented 7 years ago

2thron I am fine with the big bang approach ... :-)

Oetiker+Partner AG tel: +41 62 775 9903 (direct) Fritz Zaucker +41 62 775 9900 (switch board) Aarweg 15 +41 79 675 0630 (mobile) CH-4600 Olten fax: +41 62 775 9905 Schweiz web: www.oetiker.ch

thron7 commented 7 years ago

@pcdummy But you are aware of qx.Bootstrap.bind and qx.lang.String.startsWith, are you?!

jochumdev commented 7 years ago

I am, i just like function() {}.bind(this) more. About the startsWith it would be easy replacement, yes.

thron7 commented 7 years ago

Regarding my suggestion to have library-level config files.

I understand the reluctance to invest time and effort in the Generator, adding features that Grunt might have out of the box or that have to be re-implemented for Grunt (or something else; I'm using Grunt here as a placeholder for a JS-based build system) later. But I guess there will be a co-existence of Grunt and the Generator for quite some time, so there is probably no way to completely avoid maintaining the Generator too.

So let me point out some of the use cases I see for my proposal:

So these are some of the use cases I see. As this would be a rather cheap modification to the Generator (as all the building blocks - parsing of Manifest files, recursively adding config files - are in place) I would offer to come up with a PR if there is interest to have it.

thron7 commented 7 years ago

@pcdummy Mh, but we use exactly this all over the place in the framework, just look at e.g. qx.bom.GeoLocation or qx.core.Wrapper?!

jochumdev commented 7 years ago

@thron7 Thanks for the chat!

For others: We have been talking about Manifest.json and declarative parts in it.

I see Gruntfile.js as a replacement for config.json and grunt as a replacement for ./generate.py. This is why i would love to have add-script and add-css in Manifest.json as well as other variables a configureable library needs.

For now i can't think of a configureable library but maybe you guys have ideas/real world examples?

jochumdev commented 7 years ago

Updated my Initial proposal.

thron7 commented 7 years ago

I'm not sure the term "configurable library" gives the right intuition. But if you use add-script and add-css you are providing declarative properties of the library (and you might call that its "configuration"). Other such properties could be the compiler optimizations to be used when compiling the library code, libraries the library depends on, or macros that are expanded in the code during compilation.

jochumdev commented 7 years ago

@johnspackman implemented @require in qxcompiler. asset-let is something we don't have yet, as well as combine-images.

For combine-images we might have a task in the library and publish the library with combined images, that should also work if we write .meta files then.

johnspackman commented 7 years ago

there is an implementation of combine-images in qxcompiler.generator.Config, but it needs to be refactored as a grunt plugin (or part of your one) because it is a build facility

asset-let is implemented though - there is already facility for macro replacement in asset strings, but the macros are not scoped to just assets. EG there is @asset 's which refer to the theme as a macro

jochumdev commented 7 years ago

Renamed grunt-qxcompiler to grunt-qx.

jochumdev commented 7 years ago

I've implemented this feature into grunt-qx.

jochumdev commented 7 years ago

I've published qxc.promise as an example how a contrib could look like.

level420 commented 7 years ago

I'm not shure what the state of this issue is. I'm setting the waiting for feedback label.

thron7 commented 7 years ago

@level420 Hi Dietrich, which kind of feedback are you looking for here?

I believe Rene has implemented his vision of Manifest.json (adding add-script and add-css keys), and maybe there is a corresponding PR somewhere. I made an alternative proposal to just add a single library-config key to Manifest.json which points to a stock generator config file, which could then hold add-script and add-css keys (among others).

Do you want people to vote for either proposal, or just Rene's? Or maybe dismiss the entire issue?

jochumdev commented 7 years ago

@thron7 @level420

@johnspackman implemented add-css and add-script into qxcompiler, not sure what to do with this issue.