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

Circular references between stache templates and ES6 modules #4

Open
dylanrtt opened this issue Oct 1, 2016 · 11 comments
Open

Circular references between stache templates and ES6 modules #4

dylanrtt opened this issue Oct 1, 2016 · 11 comments
Labels

Comments

@dylanrtt
Copy link

dylanrtt commented Oct 1, 2016

I have circular references between ES6 modules and stache templates using <can-import> which are considered to use the AMD format because of this code here:

return "define("+imports+",function(module, stache, mustacheCore){\n" +
    "\tvar renderer = stache(" + intermediate + ");\n" +
    "\treturn function(scope, options, nodeList){\n" +
    "\t\tvar moduleOptions = { module: module };\n" +
    "\t\tif(!(options instanceof mustacheCore.Options)) {\n" +
    "\t\t\toptions = new mustacheCore.Options(options || {});\n" +
    "\t\t}\n" +
    "\t\treturn renderer(scope, options.add(moduleOptions), nodeList);\n" +
    "\t};\n" +
"});";

Circular references between different module formats are not supported by SystemJS, so I am interested in making the stache templates appear to use the ES6 format.

I was able to create a modified version of this plugin that returns a source string using ES6 imports/exports, which works, but now all of the code returned by can.stache() is parsed by the transpiler which doubles load times in dev (15 seconds -> 30 seconds). That's just too long.

Would it be possible to register a template module as ES6 while completely avoiding transpilation? I tried some things with instantiate, but I wasn't able to get anything working.

Side question: Is there a way to import the magic 'module' module in ES6 like you can with AMD? I see it is passed to the scope but I don't know how important that is.

Side note: I had some issues overriding the "ext" property with my custom plugin, which is related to stealjs/steal#426. I was able to use System.config() in main.js to override it at runtime but it still misses some templates that load before that code runs (also had to do it in configDependencies to get the build working). Is there a better way to override the configuration of other packages?

I'm concerned I may have to give up on using <can-import>, but I really like being able to see the imports and usage in the same file. Recently, I discovered performance problems where having over 2000 <can-import>s on the page at once (one component with 10 imports used 200 times) introduced a lot of performance overhead. For that instance, I simply moved the imports out of the template, but the cons are starting to stack up.

@justinbmeyer
Copy link
Contributor

@dylanrtt To clarify what you mean by:

Recently, I discovered performance problems where having over 2000 s on the page at once (one component with 10 imports used 200 times) introduced a lot of performance overhead.

This means you had a component like <my-popular-component> with a template like:

<can-import from="first"/>
<can-import from="second"/>
...
<can-import from="tenth"/>
...

where <my-popular-component> was used 100 times?

That's interesting the performance degraded. I'm curious if you changed the template to:

<div from="first"/>
<div from="second"/>
...
<div from="tenth"/>
...

If the approximately the same performance cost would reoccur. I want to separate out the cost of 2000 elements vs <can-import>. As these are static <can-imports> that are not dynamically exporting anything and have no live-mustache-tags within, we can probably make the number of <can-import>s irrelevant.

@dylanrtt
Copy link
Author

dylanrtt commented Oct 3, 2016

Yes, that describes my situation. Static imports - yes.

I was able to add thousands of divs with virtually no performance cost.

@justinbmeyer
Copy link
Contributor

My guess is the performance cost comes from the additional attributes event bindings that are setup to detect if from might change. We might be able to ignore that.

@matthewp
Copy link
Contributor

matthewp commented Oct 4, 2016

One option to avoid the transpilation cost would be to translate the stache module to the System.register format. This would be compatible with your ES6 code in terms of circular deps.

However we transpile everything to AMD for production so I'm not sure if the circular refs would work at that point. Certainly could be fixed so that they do.

@dylanrtt
Copy link
Author

dylanrtt commented Oct 4, 2016

@matthewp Thanks for the response. I also tried transpiling to System.register last week while troubleshooting this problem, but SystemJS saw those modules as the "register" format which is technically different than ES6 so it has the same problem. Perhaps that could be fixed in SystemJS.

@dylanrtt
Copy link
Author

dylanrtt commented Oct 6, 2016

Update on this: It looks like updating Steal significantly improved performance, so the cost of transpiling the generated stache code is no longer a serious issue.

As for the <can-import> performance cost in the DOM, I modified my version of the steal-stache plugin to simply remove <can-import> tags that have no bindings from the template source before running intermediateAndImports(). The only gotcha was that I had to add some logic to parse them out of the source to add them to the imports.

@matthewp
Copy link
Contributor

matthewp commented Oct 7, 2016

Originally <can-import>s were removed during parsing (sounds like what you changed as well?) When we created the dynamic form we removed that. I don't think we can start doing that again, unfortunately, because you can bind to can-import's view model like:

<can-import from="somewhere" {^value}="*template" />

If you're not using this feature anywhere, though, maybe we could have some type of option that allows removing them.

@justinbmeyer
Copy link
Contributor

@matthewp I think we could detect if there's some live-parts during parsing.

@matthewp
Copy link
Contributor

matthewp commented Oct 7, 2016

We could probably detect it if the <can-import> is only using from and in that case remove the tags. Otherwise, people might be doing something weird like adding a custom attr.

@justinbmeyer
Copy link
Contributor

I also feel like there's some unnecessary nodeList and fragment stuff going on if there's no subtemplate around here:

https://github.com/canjs/can-view-import/blob/master/can-view-import.js#L44

@dylanrtt
Copy link
Author

dylanrtt commented Oct 7, 2016

I do use bindings with <can-import> in some places, so I made sure the parsing only removed the ones that only have the from attribute. The only issue I have now is that I'm adding dependencies for imports that are commented out, but that's minor.

It seems that using the version of steal I upgraded to (0.16.38 / 0.16.8), the plugin I wrote works fine in development, but has a lot of issues in the production build. The first being that it's not bundled, the 2nd that the add_bundles module was resolving to an empty object, and 3rd, stache templates are being requested individually and transpiled in the browser.

I think some of the issues have to do with the fact that the plugin file is in my project rather than in a node module. Are there some special things I have to do to make that work?

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

No branches or pull requests

3 participants