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

The plugin doesn't process virtual modules #6

Closed
dkumor opened this issue Mar 3, 2019 · 8 comments
Closed

The plugin doesn't process virtual modules #6

dkumor opened this issue Mar 3, 2019 · 8 comments

Comments

@dkumor
Copy link

dkumor commented Mar 3, 2019

Hi! The plugin looks like exactly what I was looking for, but I have not been able to make it work when embedding external modules. I have the following rollup config:

import resolve from "rollup-plugin-node-resolve";
import commonjs from "rollup-plugin-commonjs";
import externalGlobals from "rollup-plugin-external-globals";

export default {
  input: "test.js",
  output: {
    name: "test",
    file: "test.jsm",
    format: "es"
  },
  plugins: [
    externalGlobals({
      vue: "Vue"
    }),
    resolve(),
    commonjs()
  ]
};

the file test.js is the following:

import Vue from "vue";
import Vuetify from "vuetify";
Vue.use(Vuetify);

I was expecting the plugin to replace imports of 'vue' to the global variable Vue, however, the following happens:

  • If I do not have vue listed as external, it is embedded in the bundle
  • If I do have it listed as external, the generated file,test.jsm has an import at the very top: import vue from 'vue'. If I manually replace the import in the generated bundle with const vue = Vue;, the resulting bundle works without issue.
  • If I have it listed as external, but do not include the plugin, the import in the bundle changes to import Vue from 'vue';, so the plugin is doing something there...
  • The order of the plugins makes no difference
  • If I remove the import Vue from "vue" in test.js, the generated file still includes the import (vuetify also imports it)

Note that the plugin works when just importing local code, to the best of my knowledge (it worked when I removed the 'Vuetify' import).

I am unsure why exactly this is happening - my guess is that the commonjs code is processed differently, but I am not sure if that's the issue - it might be something entirely different.

@dkumor dkumor changed the title Failure when embedding CommonJS Failure when embedding node_module Mar 3, 2019
@eight04
Copy link
Owner

eight04 commented Mar 3, 2019

I guess the problem is that we don't transform proxy modules generated by CommonJS plugin. Try finding the plugin (node_modules/rollup-plugin-external-globals/index.js) and changing this line from

if (!filter(id)) {

to

if (id[0] !== "\0" && !filter(id)) {
  return;
}

@dkumor
Copy link
Author

dkumor commented Mar 3, 2019

With the given modification, the output's header changes to:

import 'vue';

@dkumor
Copy link
Author

dkumor commented Mar 3, 2019

I have included a minimal environment necessary to reproduce this here (two files above+ minimal package.json):
testenv.zip

running:

npm i
npm run build

produces test.jsm with the above issues

@eight04
Copy link
Owner

eight04 commented Mar 4, 2019

You have to use CommonJS plugin before this one.

@dkumor
Copy link
Author

dkumor commented Mar 4, 2019

It works! Thank you so much!

@eight04 eight04 changed the title Failure when embedding node_module The plugin doesn't process virtual modules Mar 4, 2019
@dkumor
Copy link
Author

dkumor commented Mar 25, 2019

Would it be possible to get a release published with the above fix?

@eight04
Copy link
Owner

eight04 commented Mar 25, 2019

The only problem is that I'm not sure if we should transform all virtual modules ignoring the exclude/include option. Technically, the transformation should apply to all files including virtual modules, or there will be an untransformed import left in the bundle.

These options might be able to speed up the build though (e.g. excluding large files).

I will push a minor release that transforms all virtual modules. If it breaks something then I guess we will have to fix rollup/rollup-pluginutils#54.

@dkumor
Copy link
Author

dkumor commented Mar 25, 2019

Thanks a lot!

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

No branches or pull requests

2 participants