-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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.context doesn't work in tests #517
Comments
Yep, it is too obscure in my opinion. If we don’t document something in howto, we don’t support it. |
I’d say any custom extensions to |
cc @cpojer for evaluating implementing |
I really don't want to add this to Jest. require.context seems like a really bad API and hard to statically analyze. We really shouldn't encourage use of this because something is "hard to type" or long. This should be solvable by a custom |
Yea, I agree with @cpojer here. |
All comments makes sense. Thanks for the discussion. |
Just had a thought: what about stuff like:
Webpack generates a context module when you pass a dynamic path like this (which is the same kind of module that can be manually created with require.context). I'm not sure what the behavior in jest is for this case, but I expect that it works, though mocking it might be inconvenient. |
Inside of tests this is fine, you can require dynamic modules as much as you'd like and sometimes that is useful. Outside of tests it is bad because those cases cannot be statically analyzed. |
Although I understand the rationale of the decision on the discussion, I thought it might be valuable to provide another use case of require.context as additional feedback: In my case I'm using create-react-app to develop components that will be imported and consumed by other projects within our organizations. Part of the requirement is localization and both with my own implementation and using react-intl I've needed to use require.context in order pass an argument to a utility function or HoC enhancer to provide a path the localization bundles for that component. Without require.context Webpack will attempt to load the bundles in a location relative to the utility function and not the component. So with require.context I can do this... i18nUtils.addBundle({
requireContext: require.context("./i18n", false),
bundleName: "BaseField"
}) ...and have the utility function merge the message like this... bundleMessages = input.requireContext(`./${input.bundleName}.${language}.json`);
merge(this.messages, bundleMessages); Based on other discussions I do understand why create-react-app doesn't and should tie itself to proprietary module loading capabilities... however, I just want to highlight another use case of require.context. One improvement that could be made would be for the linting to output a warning or even an error when require.context is used in the project source... because require.context works fine in the application, but then fails in the tests. So this is of course a symptom of a broader requirement for simple i18n support, but I came across this thread when searching for information on the error message I was seeing. |
Doesn't this mean you're bundling all locales all the time? |
Well, unfortunately yes... But that's a symptom of doing this entirely on the client-side. Our previous Spring MVC based solution handled localization bundles on the server so that the client only received bundles for the requested locale. However, in the context of what we're trying to do (which is to provide re-usable components for 3rd parties to use in custom applications accessing REST APIs of our platform) it's a something that we need to live with. The issue is somewhat mitigated by trying to ensure that we only load messages of the components that are used. A simpler but more expensive solution would be to just provide a single bundle containing all of the messages for all of the components (especially given that some of the components don't even have a localization requirement). I completely agree that loading multiple bundles is not ideal, but without having control of the server this is the best solution. Again, this problem is unlikely to be common I would assume that most people just want to build their own application. |
Is there a problem with explicitly importing them in the code? After all |
How would I pass it as a reference? Let's say for example I have If I pass either Now I get a bit hazy about whether or not there is support for some root folder symbol, and I also don't know what would happen to other projects consuming my published project with regards to explicit links. The nice thing about Maybe I'm missing something really obvious because I've been staring at this problem for so long? |
I don't mean passing a path. I mean writing imports by hand: var context = {
en: require('./i18n/en'),
fr: require('./i18n/en'),
// ...
} This doesn't rely on Webpack magic. |
Well, only that I'd have to write a shed load more code which I'd like to avoid if possible. By going through a utility it means that I don't necessarily have to update the component code when support for new locales are added (our localization team would work separately to the component team and provide localization bundles outside of development sprints so the translated bundles could arrive at any time). Abstracting away behind a utility means that I can change the utility without having to change all my components. We got to over 300 components in our last application development framework - I'm trying to keep maintenance overheads down with this approach. |
Do you have a specific proposal on how we could support it better without encouraging people to use non-standard features? |
No, unfortunately not... and just to be clear, I'm not expecting you to magic a solution out of thin air, I've just found from experience that it's useful to receive feedback. I'm not expecting require.context to be suddenly supported, I just wanted to share my experience. I'll continue to try to find a good solution to the problem and will provide an update if and when I do. I do appreciate you taking the time to answer though and provide suggestions. |
One thought that does cross my mind that I might investigate is updating my local scripts to perform the Webpack build without minification (that outputs to some directory other than build) and then have the tests import from the generated resources. There are a few obvious issues with this... I don't think the build would declare explicit exports for each component so it would be necessary to import the full build layers into the test pages (which is probably not a good thing). Also, for continuous testing during development it would be necessary to trigger a build on each source file saved ... this would also rely on the output having a static name (rather than including a checksum) in order for Jest to be able to track changes to it. It probably wouldn't be particularly fast... but essentially it would avoid the issue of testing the raw, unpacked source files. This would mean that it wouldn't matter what build system was used because you'd always be testing the final output so it wouldn't make any difference what proprietary build features the code was using... Any thoughts on that? ... it obviously feels quite "hacky", but at the end of the day it doesn't actually change what the final build output would be. UPDATE: |
The final solution I've gone with is just to have an i18n folder and place all bundles in there, this means that I can specify paths that will always be relative to my i18nUtils module. The bundle location mirrors the component location. The limitation of this approach is that consumers of our project won't be able to use the utility in their own application... however, I'll probably provide support for passing require.context to the utility as an option (albeit one that we ourselves won't use within the project so that we can make use of the test framework provided) |
FYI for anyone who encounters this issue - instead of relying on |
I wonder why jest doesn't just use an environment given. I think is not a good idea to jest running in a way, and the real app in a different way. Would be nice to run your tests through webpack exactly the same as your app instead of jest having his own special stuff. So many inconsistencies and things to keep in sync between your webpack config and jests config. For example if you include polifills you need to make sure both webpack and jest config has the same ones. In my case I have a folder of migrations and I need to import them all. I don't think would be a good practice to include each one manually as you might forget to include it when you add a new one, but anyway, this is a nice to have. |
By the way I "solve" it by not calling
|
@hackhat You cannot use |
I sometimes use webpack's
require.context
to automatically load files from the filesystem. Example:Directory structure:
index.js
:docs.js
:This example is somewhat contrived because
index.js
could just be replaced with:but when you're importing hundreds of files, this is both helpful and powerful; it's easy to write a
require.context
that treats named exports differently, for example:This pattern allows you to co-locate documentation examples with your code, which discourages documentation getting out of sync with the component.
Although this example only addresses documentation, this pattern can be useful anywhere you have a lot of files with correlated features and enforcing an export signature convention is feasible and discourages code rot.
This is all possible using
create-react-app
today. However, it all breaks when you try to run your tests (in 0.3.0-alpha), because jest (naturally) doesn't use webpack:Is
require.context
"too obscure" of a webpack feature? Shouldcreate-react-app
start disallowing it or warning about its usage? Where and how do we draw the line on which webpack features should be supported and which should not be?Or, should we try to support
require.context
in the test runner enrivonment? If so, where and how do we draw the line on which webpack features should be supported in the test environment?The text was updated successfully, but these errors were encountered: