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

Dynamic requires causes webpack to include test files in bundle #7

Open
davej opened this issue Feb 12, 2018 · 8 comments
Open

Dynamic requires causes webpack to include test files in bundle #7

davej opened this issue Feb 12, 2018 · 8 comments

Comments

@davej
Copy link

davej commented Feb 12, 2018

react-xterm contains dynamic requires which causes Webpack to be overly aggressive when bundling and include all the addons and corresponding tests:

this.props.addons.forEach(s => {
  const addon = require(`xterm/dist/addons/${s}/${s}`);
  Terminal.applyAddon(addon);
});

Causing the following error:

* chai in ./node_modules/xterm/dist/addons/attach/attach.test.js, ./node_modules/xterm/dist/addons/fit/fit.test.js and 4 others
* express in ./node_modules/xterm/dist/addons/zmodem/demo/app.js
* express-ws in ./node_modules/xterm/dist/addons/zmodem/demo/app.js
Caused by this dynamic require which causes webpack to bundle all the addons and their tests:

Perhaps the signature of this.props.addons can be an array of the addon modules directly, rather than an array of strings which are converted into dynamic imports?

@farfromrefug
Copy link
Owner

@davej i see the webpack issue.
I would prefer not to maintain an array. That way whatever plugin are integrated with xterm, it will simply work.

Couldn't you add a rule in webpack to ignore test files from addons? This is what i do when packing for electron

@davej
Copy link
Author

davej commented Feb 12, 2018

I would prefer not to maintain an array.

No, you don't need to maintain a list of plugins, the plugin would be imported outside of the react-xterm lib. Something like this:

import fit from "xterm/dist/addons/fit/fit";
import { XTerm } from "react-xterm";

export default () => (<XTerm addons={[fit]} />);

Couldn't you add a rule in webpack to ignore test files from addons?

I tried the following, but it didn't seem to work (possibly because it's not a direct dependency of my project?):

 externals: [
  "node-pty", "xterm"
]

If I add react-xterm to the array then it works but I end up getting the error seen in #6 instead.

@farfromrefug
Copy link
Owner

@davej ok i see what you mean. Perso i don't want this. The point of the addon props is not to have to do the import, nor to care about the actual path.

What about this?
https://stackoverflow.com/questions/25834396/how-can-i-exclude-code-path-when-bundling-with-webpack-browserify

@davej
Copy link
Author

davej commented Feb 12, 2018

I tried a few similar things as in that link but couldn't get it to work. I ended up basically doing a copy/paste of react-xterm and I'm maintaining it as a local component instead. I'd also prefer not to make xterm an external library if it doesn't need to be.

Feel free to close this if it doesn't fit in with your usage. I can maintain this as a local component, it's not a big problem, 👍.

@farfromrefug
Copy link
Owner

@davej i don't want you to maintain it as a local component :D This is not the point of open sourcing :D
Even if i don't really want to change that prop for now, i want it to work for you

It seems a little bit tricky with webpack but doable:
https://stackoverflow.com/questions/25384360/how-to-prevent-moment-js-from-loading-locales-with-webpack/25426019#25426019
webpack/webpack#198 (comment)

I would like to help you more, but without knowing the layout of your project or your webpack file, it's a bit tricky

@davej
Copy link
Author

davej commented Feb 12, 2018

My webpack config is:

{
    target: "electron-renderer",
    externals: ["node-pty"]
}

I'm using electron-next. I'll give that webpack plugin a quick try now and see if it works for me.

@Grsmto
Copy link

Grsmto commented Mar 23, 2018

Hi guys,
Fyi I had a similar issue and I used the solution you suggested using webpack.IgnorePlugin.
Here is my config: new webpack.IgnorePlugin(/\.(test|spec)\./).
However I had a context issue in my dynamic import as well!
My import statement was like that:

import(`../../pages/${page}`)

and it worked only after adding the file extension like so:

import(`../../pages/${page}.js`)

Hope this help.

@lukevp
Copy link
Collaborator

lukevp commented Jun 2, 2020

I'm guessing this is no longer an issue in the new RC I am building since xterm no longer bundles addons but will test & close as appropriate.

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

4 participants