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

research on conditional/when after and before #1718

Open
caridy opened this issue Mar 19, 2014 · 5 comments
Open

research on conditional/when after and before #1718

caridy opened this issue Mar 19, 2014 · 5 comments

Comments

@caridy
Copy link
Member

caridy commented Mar 19, 2014

Part of the alignments with ES modules will require some adjustments in our current conditional loading strategy. As today, YUI Core only uses after and instead as values for when property when defining a conditional module, it doesn't use before although we have tests for them, and there might be some people using it.

The problem is that in ES there is not after and before, only instead, since each module is suppose to export something, and you should not mix the export with more things that happen after or before.

After looking at the metas today, it seems like the use of after is hacky, and could be solved by implementing instead, and having a more granular approach for the things that are common for multiple modules.

Here is the current meta using after:

    "yql-jsonp": {
        "condition": {
            "name": "yql-jsonp",
            "test": function (Y) {
                  /* Only load the JSONP module when not in nodejs or winjs
                  TODO Make the winjs module a CORS module
                  */
                  return (!Y.UA.nodejs && !Y.UA.winjs);
            },
            "trigger": "yql",
            "when": "after"
        },
        "requires": [
            "jsonp",
            "jsonp-url"
        ]
    },
    "yql-nodejs": {
        "condition": {
            "name": "yql-nodejs",
            "trigger": "yql",
            "ua": "nodejs",
            "when": "after"
        }
    },
    "yql-winjs": {
        "condition": {
            "name": "yql-winjs",
            "trigger": "yql",
            "ua": "winjs",
            "when": "after"
        }
    }

Ideally, we could have a yql-base that implements the shared pieces between all these 3 modules, and then having yql, yql-winjs and yql-nodejs to depend on it, then having yql-winjs as a replacement for yql when in winjs, and the same for the nodejs version. That's a more elegant solution, and we could finally kill the when property, implementing the instead path for all conditionals.

In the case of the yql-jsonp, this really qualifies as an optionalRequires, specially if it depends on yql-base. Where yql is just a simple shim around the yql-base implementation.

Here is the proposed meta:

    "yql-base": {}
    "yql": {
        "requires": ["yql-base"],
        "optionalRequires": ["yql-jsonp"]
    },
    "yql-jsonp": {
        "test": function (Y) {
                  /* Only load the JSONP module when not in nodejs or winjs
                  TODO Make the winjs module a CORS module
                  */
                  return (!Y.UA.nodejs && !Y.UA.winjs);
        },
        "requires": [
            "jsonp",
            "jsonp-url"
        ]
    },
    "yql-nodejs": {
        "condition": {
            "name": "yql-nodejs",
            "trigger": "yql",
            "ua": "nodejs"
        }
    },
    "yql-winjs": {
        "condition": {
            "name": "yql-winjs",
            "trigger": "yql",
            "ua": "winjs"
        }
    }

/cc @juandopazo

@juandopazo
Copy link
Member

We should probably have this conversation in yui-contrib.

@caridy
Copy link
Member Author

caridy commented Mar 19, 2014

sure thing. /cc @triptych

@triptych
Copy link
Contributor

Yeah, I'd say just kick off a thread in yui-contrib to point to this issue, so you can capture the discussion here.
This is similar to how Satyen worked on the YUI performance work he did.

@caridy
Copy link
Member Author

caridy commented Mar 19, 2014

@caridy
Copy link
Member Author

caridy commented Mar 20, 2014

Related to issue #1661 and PR #1704

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

3 participants