More build issues #312

Closed
korylprince opened this Issue Jan 10, 2017 · 3 comments

Projects

None yet

2 participants

@korylprince
Contributor

@marcosmoura You're going to hate me :(

So with the change you just made to externals, now using Vue and VueMaterial from <script> tags is broken. I've done some more research and I (think) know what the problem is, but not how to fix it (yet.)

The externals appears to search two places: in a global javascript variable, and for an imported package.

When externals is {vue: Vue} building with webpack fails because there is no global variable Vue (since webpack hides globals behind functions)

When externals is {vue: vue} building succeeds because vue resolves to a module. But now a <script> import fails because vue is not the global variable it expects, Vue is.

So either more advanced externals configuration is required, or there needs to be something else configured to support both. I've looked at vue-router, vuex, and vuefire as examples, but none of them seem to use webpack to build.

Here's the current error using <script> tags:

vue-material.js:6 Uncaught TypeError: r.default is not a constructor

Inserting a script between vue and vue-material's script tags with var vue = Vue; fixes the issue, but obviously that's not a good solution...

@korylprince
Contributor
korylprince commented Jan 10, 2017 edited

I now have a working solution, that I believe is the correct solution.

The webpack docs leave much to be desired but after reading full configuration docs here is the syntax that works for both module import and script tags:

  externals: {
    vue: {
        commonjs: "vue",
        commonjs2: "vue",
        amd: "vue",
        root: "Vue",
    }
  }

When using export type umd, we can specify the external name for each type, allowing us to normally use vue for module imports, and Vue for script tag usage.

Does this make sense? I can now understand the hate javascript build environments get... Things never seem to be as simple as they first appear. I apologize for giving you a half-baked solution last time.

@korylprince
Contributor

Also, one other question which I think is related. In index.js, you set window.VueMaterial.

I can't see anywhere in the entire source where you ever use it again; It looks like you coded this very close to the first release.

This doesn't seem to be a common practice among other Vue plugins. It defeats one of the purposes of using webpack and modules: keeping things contained. Webpack and friends take care of getting VueMaterial to the right place.

My guess is that this is just a forgotten piece of code that should probably be removed since your current webpack takes care of making that variable available.

@korylprince korylprince added a commit to korylprince/vue-material that referenced this issue Jan 10, 2017
@korylprince korylprince fix externals for #312 dcd544f
@korylprince korylprince added a commit to korylprince/vue-material that referenced this issue Jan 10, 2017
@korylprince korylprince remove window.VueMaterial for #312 bd44324
@marcosmoura
Owner

The externals trick worked flawlessly!
The window variable was created to work with the standalone build, to use the Vue.use(VueMaterial).
But with the new externals this become obsolete:

  externals: {
    vue: {
      commonjs: 'vue',
      commonjs2: 'vue',
      amd: 'vue',
      root: 'Vue',
      var: 'Vue'
    }
  }

Thanks a lot @korylprince!!!

@marcosmoura marcosmoura added the bug label Jan 10, 2017
@marcosmoura marcosmoura added a commit that referenced this issue Jan 10, 2017
@marcosmoura fix standalone build #312 aa20425
@marcosmoura marcosmoura added a commit that referenced this issue Jan 10, 2017
@marcosmoura fix standalone build #312 834c5c3
@marcosmoura marcosmoura added a commit that referenced this issue Jan 11, 2017
@marcosmoura Merge remote-tracking branch 'origin/master' into develop
* origin/master:
  [release] 0.6.3
  [build] 0.6.3
  fix standalone build #312
95b32ff
@marcosmoura marcosmoura added a commit that referenced this issue Jan 11, 2017
@korylprince @marcosmoura korylprince + check for null parent elements in mdInkRipple #313 (#317)
* fix standalone build #312

* [build] 0.6.3

* [release] 0.6.3

* fixes #313 check for null
d66609f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment