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

@inlineView dependencies not bundled #107

Closed
eikaramba opened this issue Apr 12, 2017 · 11 comments
Closed

@inlineView dependencies not bundled #107

eikaramba opened this issue Apr 12, 2017 · 11 comments
Assignees

Comments

@eikaramba
Copy link

eikaramba commented Apr 12, 2017

I guess this is definitely a webpack-plugin issue ;)

In aurelia-materialize-bridge one of the view components is using an @inlineView Template which requires a css file (see this issue aurelia-ui-toolkits/aurelia-materialize-bridge#401 and here the code https://github.com/aurelia-ui-toolkits/aurelia-materialize-bridge/blob/master/src/slider/slider.js)

@inlineView(`
  <template class="slider">
  <require from="./slider.css"></require>
  <ul class="slides">
    <slot></slot>
  </ul>
  </template>
`)

Unfortunately the slider.css dependency is not bundled, is that not handled yet?

@Thanood
Copy link

Thanood commented Apr 12, 2017

Sorry, I just changed that component to an html/js pair.
Here is the code as it was before:
https://github.com/aurelia-ui-toolkits/aurelia-materialize-bridge/blob/89d949bad5badb9e41fa3145fec5cb054609e589/src/slider/slider.js

@jods4
Copy link
Contributor

jods4 commented Apr 12, 2017

Yes indeed.

Not sure how to handle that, probably evaluate the string parameter of inlineView functions and run that through the html-requires-loader code.

@Thanood
Copy link

Thanood commented Apr 13, 2017

Btw. after using moduleName it seems like all the custom elements are bundled but no custom attributes.

Here's a screen of the trace output from @eikaramba in the issue mentioned above:

image

These are all custom elements while attributes like autocomplete or badge are missing from the bundle.

Of course I can create a separate issue for that.

@jods4
Copy link
Contributor

jods4 commented Apr 13, 2017

@Thanood custom attributes are generally registered either:

  • locally with <require from=".."> which should work if it's in a .html file.
  • globally with a call to .globalResource() where you need to add a PLATFORM.moduleName() around the parameter. If this is inside a 3rd party-lib that you can't modify, ModuleDependenciesPlugin is a work-around.

There is no specific knowledge of custom elements or custom attributes inside the Webpack plugin.

@Thanood
Copy link

Thanood commented Apr 13, 2017

@jods4 Thank you for your help and sorry for not being precise.. 😃
In fact, I can modify the library in question.

This is the src directory of aurelia-materialize-bridge (only if you want an overview): https://github.com/aurelia-ui-toolkits/aurelia-materialize-bridge/tree/master/src

The globalResources are built using a builder pattern in config-builder.

For example the auto-complete custom attribute is added like this:

  useAutoComplete(): ConfigBuilder {
    this.globalResources.push('./autocomplete/autocomplete');
    return this;
  }

The config-builder creates an array which is used in index.js. It is mapped to provide PLATFORM.moduleName() to each array item defined in config-builder.

This is then loaded as an aurelia plugin..

export function configure(aurelia, configCallback) {
  
  // ...

  if (builder.useGlobalResources) {
    const mappedResources = builder.globalResources.map(r => PLATFORM.moduleName(r));
    aurelia.globalResources(mappedResources);
  }
}

While this approach seems to work for custom elements as well as html-only components, it seems to fail for custom attributes (f.i. the auto-complete attribute, which is missing from the trace output above).

Of course I may have done something wrong but I don't see it. 😃

Just in case you want to try it yourself: This version is not yet published on npm but can be installed from its master branch.

I will try this again if I have some more time. 😃

@jods4
Copy link
Contributor

jods4 commented Apr 13, 2017

You can't globalResources.map(PLATFORM.moduleName).

moduleName is analyzed statically at build-time by Webpack to determine what needs to be included in your bundle. Dynamic code like shown above can't be analyzed.

You should wrap the strings when you push them:

this.globalResources.push(PLATFORM.moduleName('./autocomplete/autocomplete'));

Caveats:

  • Since it's a relative path, you need to do the push in the same folder as the module that will ultimately call .globalResource() at runtime.
  • Even though users might not call useAutoComplete() and ultimately not use ./autocomplete/autocomplete the module will still be included in the bundle.

That second point is implied by the fact that bundle contents are determined at build-time and there's no way Webpack can analyze JS to determine what code will run or not (so it assumes everything runs and every moduleName that is found gets included in the bundle).

If your goal is to be able to include in the bundle just the modules that are used, you'll need alternative strategies.

One way is to repeat the configuration in the webpack config. Ultimately you'll want:

new ModuleDependenciesPlugin({
  'aurelia-materialize-bridge': ['./autocomplete/autocomplete', /* parts you actually use */ ]
})

This is how Aurelia does it for .use.standardConfiguration() and friends, see aureliaConfig option.

Another way could be to have each moduleName in a separate module, maybe so:

import { resources } from 'aurelia-materialize-bride';
resources.push(PLATFORM.moduleName('./autocomplete/autocomplete'));

And then ask users to create a module that lists what they want to use:

import "aurelia-materialize-bridge/autocomplete";
import "aurelia-materialize-bridge/..."; // etc.

Since Webpack traces dependencies precisely, only the required modules end up included.

@Thanood
Copy link

Thanood commented Apr 13, 2017

@jods4 Thanks a lot for the thorough explanation. 👍

I need to think about that because it's potentially changing my structure or even install process.
For now, I will use moduleName when I push the modules.

Isn't it interesting that it actually works with custom elements?

@jods4
Copy link
Contributor

jods4 commented Apr 13, 2017

It is surprising! Maybe they're referenced by another mean?

@Thanood
Copy link

Thanood commented Apr 13, 2017

Not that I know of..
They're exported but attributes are, too. I don't know.. 😃

@niieani
Copy link
Contributor

niieani commented Apr 13, 2017

@jods4 awesome explanation here. We should probably add it to your wiki (and docs).

@jods4
Copy link
Contributor

jods4 commented Jun 18, 2017

RC-3 contains code that tries to detect resources inside @inlineView('<template>').

The configuration (which tags/attributes) is shared with html-requires-loader. The only thing that is new is recognizing inlineView function calls.

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