Parsing of JSON imports

[kirlat]

Since inflection-tables tests are moved to use babel-jest, there is an issue with parsing of JSON imports. Tests are executed in node.js environment, and statements like import file from 'file.json' results in file being in object.

However, mostly due to historical reasons (since Rollup which we used before did not parse JSON files automatically), we import JSON files as strings and parse them manually with JSON.parse().

That creates problems with testing, since in tests we have imports as objects (since it is parsed automatically by node.js), but our code expects it to be a string (as during "normal" execution in browser environment). I did not find any way to disable automatic parsing of JSON imports by node.js (if you know any, please let me know!).

So it seems that the only solution is to enable automatic parsing of JSON imports by Webpack. This way it will match node.js behavior during testing and the problem will go away.

What do you think? Do you see any other ways to solve this?

[irina060981]

Hello, @kirlat and @balmas

I have made a little investigation and find out this:

All JSON files are imported with JSON.parse from jest-runtime
You could see it here - node_modules\jest-runtime\build\index.js

That is why It couldn't even be transformed to a string using jest/transform config option (as I did for image files in components), I have tried - but it ignores transform for json files.

I have found a discussion about it on the forum of facebook\jest -
jestjs/jest#2578

And developers said that they won't change it even if it so not suitable for everyone.

When I have faced with such a case at the components tests - I have used JSON.stringify for all json files - it will have the same result as for strings or object.

Hope, my information will be usefull.

[kirlat]

Irina, thank you for your information! A comment by Christopher states clearly that it is done to emulate node.js behavior and it won't change anytime soon most likely. Simon offers a patch for Jest runtime, but I'm not sure it's worth it for us.

So I would offer for discussion the following question:
Would it be simpler and overall better for us to change Webpack settings so that all JSON imports be parsed automatically? Then it will mimic behavior of node.js and Jest (which follows the lines of node.js). And thus there will be no need to stringify objects for testing.

I don's see any use cases where we would need to have JSON imports unparsed and parse them manually. Do you?

Do you think this change, if made globally to all libraries, will simplify things for dev and especially for testing?

[irina060981]

As for me - I didn't see all the code - but I didn't face with cases that needs manual parsing.
As I saw - only our own json's files are parsed, and if it will be a need to fix some bugs in json, it wouldn't be very difficult to change it.
It is not really difficult to stringify JSON in test's files - but I think that it will be important to have the same behaviour in the code and in the tests that check this code.

[irina060981]

Also I have started to create tests for inflection tables and as for paradigm tables - it uses JSON.parse in verbParadigmTables and verbParticipleParadigmTables - and it couldn't be executed with Jest as it imported json as object. So I couldn't solve it with trandform and with direct JSON.stringify - only way is synchronize the way of handling json in production and test environments.

[balmas]

Sorry for chiming in late here, but I'd like to see us handle the imports consistent across test and production environments. So if changing Webpack is the simplest way to do that, then I'm in favor of it.

[kirlat]

We can update all libraries at once, but we can also do it one by one. Here is what I think would work:

The current version of node-build is 0.0.7. With it, Webpack does not parse JSON. I can tag it with v0.0.7. Then I can make a 0.1.0 (because it's a breaking change) version of node-build that will make Webpack to parse JSON.

So those libraries that are not updated (i.e. still parses JSON manually) should update their package.json to require v0.0.7 of node-build. The ones that will have manual parsing removed (i.e. updated ones) should just use the latest version of node-build instead. This way all should work and we'll be able to update libraries gradually, as needed. What do you think?

[balmas]

yes I agree with that suggestion.

[kirlat]

That all should be in master now

[irina060981]

I believe it could be closed.