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

Engine: support .pragma library for js files #377

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ChALkeR
Copy link
Member

@ChALkeR ChALkeR commented Dec 26, 2016

Fixes: #375

Testcase is not ready yet.

/cc @stephenmdangelo for #255 and qmlweb/qmlweb-parser#26 — perhaps you would be interested to try this out =).

Copy link
Member

@stephenmdangelo stephenmdangelo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will match Qt QML behaviour for Qt.include, as that should create a copy of the included file, ignoring any pragma statements within it: http://doc.qt.io/qt-5/qtqml-javascript-imports.html#including-a-javascript-resource-from-another-javascript-resource

When importing a JavaScript file into a QML file, wouldn't it be faster/easier to keep a global reference to the context object that is created the first time it is imported (e.g. Whatever is assigned to the "as" variable, as opposed to the "contextSetter" function), and then create a variable with the qualified name inside of the Component context and assign the global object to it?

I think that would require changes outside of loadJS.

@ChALkeR
Copy link
Member Author

ChALkeR commented Dec 31, 2016

@stephenmdangelo Thanks.

I will check what could be done with Qt.include. I don't think that there are any performance issues from an additional closure here, at least not until the benchmarks have proven that, though.

I will try to do the other impl (like you proposed) and will compare the two patches on size and complexity (given that Qt.include support would be included into both).

@stephenmdangelo
Copy link
Member

I think for Qt.include, just maintain the old behaviour and ignore the pragma. That's why I think this feature has to go up a level or two to wherever "importJavascriptInContext" is called.

It's less the performance (unless a library has a million exports or something), and more just the simplicity. A global object per library that is just referenced by different names in the context of each QML file.

In

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants