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

--require not working as expected #1

Closed
randycoulman opened this issue Mar 22, 2016 · 10 comments
Closed

--require not working as expected #1

randycoulman opened this issue Mar 22, 2016 · 10 comments

Comments

@randycoulman
Copy link
Contributor

Thank you so much for this package! It is making my testing workflow so much better!

I'm running into an issue with --require. I have the following mocha-webpack.opts file:

--colors
--require client/__tests__/specHelper.js
--reporter spec
--webpack-config webpack.config.js
--glob *-spec.js
--recursive
client

My specHelper.js file is being required properly, but its effects are not being made visible to my tests when they run.

specHelper.js looks like this:

const chai = require('chai')
const chaiAsPromised = require('chai-as-promised')
const sinonChai = require('sinon-chai')

chai.use(chaiAsPromised)
chai.use(sinonChai)

When I run my tests, I get the error: TypeError: (0 , _chai.expect)(...).to.become is not a function, which means that chai-as-promised is not available.

When I previously ran my specs using mocha directly, the --require option worked as I expected.

If I rename specHelpler.js to specHelper-spec.js, then it works as I expect; however, if I run mocha-webpack with the --watch option and then change a test, I get the same error message again because the specHelper-spec.js file wasn't reloaded by --watch.

At this point, the only workaround I can think of is to explicitly import my spec helper into all of my test files.

I've had a look at the code, and I don't see any obvious problems with it. Somehow it seems like the webpack-compiled code doesn't have access to the chai object that was configured by the spec helper.

randycoulman added a commit to CodingZeal/react-boilerplate that referenced this issue Mar 25, 2016
There’s an issue with mocha-webpack where requiring the spec helper in
the config file doesn’t make its changes visible to the actual specs.
See zinserjan/mocha-webpack#1.
@zinserjan
Copy link
Owner

Somehow it seems like the webpack-compiled code doesn't have access to the chai object that was configured by the spec helper.

Yep, that's the reason. This issue is caused by webpack when node_modules are not excluded from bundling. Then you have 2 versions of your node_modules and webpack uses it's own bundled version...

If I rename specHelpler.js to specHelper-spec.js, then it works as I expect; however, if I run mocha-webpack with the --watch option and then change a test, I get the same error message again because the specHelper-spec.js file wasn't reloaded by --watch.

This includes your specHelper to webpacks bundle and affects the used modules for the initial run. But when you change a file, webpack rebuilds the bundle and only those files which are affected by this change will be executed again.


Excluding node_modules feels more like a workaround, but it's the simplest solution for this issue. Have a look at webpack-node-externals to exclude them. The bundle process is also faster with this plugin ;)

For a real solution, we need to make sure that node & webpack shares their instances of modules - when needed.

@randycoulman
Copy link
Contributor Author

I originally tried using webpack-node-externals as you show in the README, but ran into other issues. I can't remember what they were off the top of my head, but I'll re-try that when I get a chance and report back.

@randycoulman
Copy link
Contributor Author

OK, I've had a chance to re-investigate, and the problem was unrelated to this module. We're using eslint-plugin-import, and when we use webpack-node-externals, the import/no-unresolved rule complains about every import statement we have in our codebase.

randycoulman added a commit to CodingZeal/react-boilerplate that referenced this issue Mar 28, 2016
While the [issue I opened on
mocha-webpack](zinserjan/mocha-webpack#1) is
still outstanding, the author’s suggestion is to use
`webpack-node-externals`.  We tried that initially, but that results in
numerous firings of the `import/no-unresolved` lint rule.

After conferring with @lexun, we decided to go back to that approach
and instead disable the linters when doing `npm run test`.  They still
run with `npm run build`, `npm run test:debug`, and `npm run dev`.

If the above-mentioned issue gets resolved, then we can go back to
running the linters in all environments.
@randycoulman
Copy link
Contributor Author

@zinserjan Do you have any ideas on how to accomplish what you said above?

For a real solution, we need to make sure that node & webpack shares their instances of modules - when needed.

I'm happy to try to work on a PR for this, but I'm not sure where to start with it. If you could point me in a direction, I'll see what I can do.

@zinserjan
Copy link
Owner

Thought a lot about this the last days and I'm still not sure how to solve it...

The main problem is, that webpack creates it's own js bundle with all necessary modules. If we use --require some-code.js then "some-code.js" will be executed inside node's environment and all enhancements to modules are only applied in this env. Everything works fine until webpack bundles some of the used dependencies in "some-code.js" into it's own bundle.
Then the enhancements to this dependencies are lost, cause webpack uses it's own version/instance of this module... This behavior breaks your code when it depends on some enhancements like helper methods for chai.

I see the following solutions for this problem:

  1. exclude used dependencies in --require files from webpack's bundle
    • dependencies are managed by node --> shared instances
  2. include all --require calls also to webpack's bundle
    • code executes just twice --> nothing shared, but should work in the most cases
    • could lead to side effects when you need a shared or non-changing instance of a module
  3. provide another command line option (--include) to prepend all supplied files to webpack's bundle.
    • --require can be used for run-time enhancements like mocha-clean
    • --include can be used to include some files into webpack's bundle like test helpers

The more I think about the first solution the more I come to the result, that there will probably never a bullet-proof solution for this. In theory it would be enough to externalize all used dependencies in -require files. But to implement this, we have to enhance the (existing) webpack config with some externals and this could break easily when there is already a externals helper defined that interact in some other way...

Second way feels more a like a hack than a solution.

Third is probably the way to go. It just includes the code into webpack's bundle, so it just works for use cases like spec helpers. Shared instances are also possible - with a custom externals configuration.
And for run-time enhancements there's still the --require option.

What do you think?

@randycoulman
Copy link
Contributor Author

My knowledge of how webpack works is pretty weak, but I think option 3 sounds best as well. What's not clear to me is whether the --include option will continue to work when using --watch. If I --include my spec helper that configures some chai plugins, will those continue to be available in subsequent test runs, even if the spec helper doesn't get reloaded? If the answer is yes, then --include addresses my use case.

I don't have enough knowledge or understanding of webpack to know whether there will be other undesirable effects of doing this.

I can try to come up with something along these lines when I get some open-source time, but if you see a clear path to implementing this, then go for it.

@zinserjan
Copy link
Owner

If I --include my spec helper that configures some chai plugins, will those continue to be available in subsequent test runs, even if the spec helper doesn't get reloaded?

Yes! That's the idea.

Added a PR for this. But before I'll merge this, I wanna test it on windows and try it with a real project.

@randycoulman
Copy link
Contributor Author

I just tried out your PR branch on one of my projects, and it worked perfectly. Thanks for that! I don't have a Windows setup to test on, but consider it successful on a real project!

@zinserjan
Copy link
Owner

Cool. Windows looks good, too.

Released as 0.3.0

@randycoulman
Copy link
Contributor Author

Updated all of my affected projects, and everything works great! Thanks so much for solving this issue!

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