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

Add AMD config to AppManifest #160

Open
montlebalm opened this issue Apr 3, 2014 · 7 comments
Open

Add AMD config to AppManifest #160

montlebalm opened this issue Apr 3, 2014 · 7 comments

Comments

@montlebalm
Copy link
Member

In order to support dependency aliasing, we'll need some way for the app to specify it's AMD dependencies. I think a new property on the AppManifest could solve this nicely. For example:

{
    apps: [],
    inlineScripts: [],
    scripts: [],
    styles: [],
    scriptAliases: {
        moment: "//cdnjs.cloudflare.com/ajax/libs/moment.js/2.5.1/moment.min"
    }
}

Upon loading an app, we would pass the dependencies property into the paths property of the AMD config. This would help prevent script duplication and would allow for multiple versions of a library to exist simultaneously.

I think there are a few outstanding questions here:

  1. What should the property be called? Dependencies? Aliases? Paths?
  2. Should we replace the old property scripts with this new object?
  3. Is this method of defining dependency aliases universal to AMD or is it just for RequireJS?
@montlebalm montlebalm added the v2 label Apr 3, 2014
@markhealey
Copy link
Member

  1. Hold on a sec
  2. No, I don't think so.
  3. Yes, universal to AMD

I don't think we can limit the scope of proposed scriptAliases in the manifest to just aliases because of the flexibility and multitude of props in both the require.config or dojoConfig. How do apps provide more detailed configs (for situations where baseUrl or shim configs are needed)? I guess the baseUrl could be a container-specific option, and dojo doesn't have a shim the way requirejs does (it calls for the use! plugin). Perhaps aliases is as far as we want to go...

@brianbaker
Copy link
Member

  1. Call it paths to match CommonConfig paths
  2. This is an interesting question. On one hand you need some way to tell F2 where your AppClass comes from. Once F2 knows that, there is no reason to have manifest.scripts because the app can just put the scripts as dependencies: define('com_example_a', ['my.js', 'scripts.js'], function() { ... }); So there isn't as much a need for manifest.scripts as there is a need to define where your app class is.
  3. I think its universal to CommonConfig but AMD loaders are not required to implement paths. Ultimately, I think F2 will just need to test out the various loaders out there and approve them for support (RequireJS & Dojo).

@montlebalm
Copy link
Member Author

  1. I think "paths" is a good name based on AMD, but I thought we might want to make a little more generalized in case we move off AMD down the road or if we end up supporting non-AMD apps.
  2. One possibility is to have "paths" and a "main" property that points to the actual app definition. We could call that property "script", "main", "classFile", or anything else that makes sense.

@brianbaker
Copy link
Member

What if scripts could be either an object (CommonConfig paths) or an array of strings?

{
    scripts: {
        "com_example_a": "//example.com/js/com_example_a",
        "jquery-1.11.0": "//ajax.googleapis.com/ajax/libs/jquery/1.11.0/jquery.min"
    }
}
{
    scripts: [
        "//example.com/js/com_example_a.js",
        "//ajax.googleapis.com/ajax/libs/jquery/1.11.0/jquery.min.js"
    ]
}

@montlebalm
Copy link
Member Author

The current downside is that the app wouldn’t be able to tell us which script should be used to load the app class. I suppose they could alias it after their appId, but it doesn’t sound great. I previously suggested that we could add a “main” property that would point to the AppClass.

The other issue here is that many scripts also require a “shim” to work with AMD. We can support a property for that as well, or look for a more general kind of AMD config. There are certainly problems with that (e.g., “baseUrl”), so we’ll need to consider many possibilities.

On April 7, 2014 at 8:31:08 PM, Brian Baker ([email protected]) wrote:

What if scripts could be either an object (CommonConfig paths) or an array of strings?

{
    scripts: {
        "com_example_a": "//example.com/js/com_example_a",
        "jquery-1.11.0": "//ajax.googleapis.com/ajax/libs/jquery/1.11.0/jquery.min"
    }
}
{
    scripts: [
        "//example.com/js/com_example_a.js",
        "//ajax.googleapis.com/ajax/libs/jquery/1.11.0/jquery.min.js"
    ]
}

@brianbaker
Copy link
Member

Well, an app has to provide a script that defines a module named the same as its AppId, so the more I thought about the main property the more I wondered if F2 needed to know exactly what file it came from. Eventually, the code does a require(appIds), so by that point it the app will have had to either been defined via the scripts object (paths) or else required via a scripts array.

@ilinkuo
Copy link

ilinkuo commented Apr 9, 2014

I think that instead of duplicating parts of CommonConfig inside of AppManifest, why not just put the whole thing in>

{
    apps: [],
    inlineScripts: [],
    scripts: [],
    styles: [],
    commonConfig: { // or name it simply config
        baseUrl: //...
        packages: //...
    }
}

And I also like @brianbaker's idea of require(appIds)

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

4 participants