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

fix #394 tplScript unnecessarily extending Template #395

Closed
wants to merge 1 commit into from

Conversation

jakub-g
Copy link
Collaborator

@jakub-g jakub-g commented Feb 26, 2013

If a template had a CSS template with script in its dependencies, then
a circular dependency error might have happened due to that.

Close #395.

@benouat
Copy link
Member

benouat commented Feb 27, 2013

I really don't get the change here....
I already worked with Templates that have CSSTemplates with scripts and never ever faced this issue.

Moreover, i don't get at all the new Error Message

Aria.CONSTRUCTOR_IN_TPLSCRIPT = "Error: constructors should not be used in template scripts";

What does it mean ? We should never declare or use a constructor in a tplScript ????

@divdavem
Copy link
Member

The error message should probably be something like: "A template script should not be instanciated directly."
This change does not remove the ability to have a constructor in a tplScript.

@divdavem
Copy link
Member

@benouat The circular dependency which is mentioned in this issue is very specific.
It appears when trying to add aria.widgets.GlobalStyle as a dependency of aria.widgets.Widget. The cycle passes through aria.tools.contextual.ContextualMenu (conditional dependency of aria.templates.Template, which should probably be also removed) which depends on aria.widgets.Template, which extends aria.widgets.Widget.

@jakub-g
Copy link
Collaborator Author

jakub-g commented Feb 27, 2013

I've fixed a message and added some explanatory comments to the file. The situation is exactly how @divdavem described it earlier.

@benouat
Copy link
Member

benouat commented Feb 27, 2013

@jakub-g ok, it's clearer, probably the original issue #394 has been poorly named, tplScript unnecessarily extending Template seems to have been way too generic for me to understand it correctly in the first place

If a template had a CSS template with script in its dependencies, then
a circular dependency error might have happened due to that. This commit
removes the the dependency of a tplScript on aria.templates.Template
which could lead to that situation.

Additionally, the $constructor and $destructor functions are removed
at Aria.tplScriptDefinition call time to prevent direct instantiation of
tplScripts (but the user can still specify $constructor and $destructor
in their template script and they will be invoked).

Close ariatemplates#395.
@jakub-g
Copy link
Collaborator Author

jakub-g commented Feb 27, 2013

I've renamed the #394 and added some more info in the issue and the commit details. However I'm not sure if we can provide a better commit title in 3-4 words.

@piuccio piuccio closed this in b3b3c80 Mar 7, 2013
@piuccio
Copy link
Contributor

piuccio commented Mar 7, 2013

Integrated in b3b3c80

@jakub-g jakub-g deleted the iss394 branch December 13, 2013 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants