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

required JSON breaks test #45

Closed
vidartf opened this issue Dec 7, 2016 · 6 comments
Closed

required JSON breaks test #45

vidartf opened this issue Dec 7, 2016 · 6 comments

Comments

@vidartf
Copy link
Contributor

vidartf commented Dec 7, 2016

Having a line like

const data = require('path/to/data.json');

will give an error during testing:

Chrome 55.0.2883 (Windows 10 0.0.0) ERROR
  Uncaught SyntaxError: Unexpected token m in JSON at position 2
  at http://localhost:9876/context.html:2

See test repo: https://github.com/vidartf/karma-ts-test/tree/json
Travis showing error: https://travis-ci.org/vidartf/karma-ts-test/jobs/182115047
Appveyor showing other error: https://ci.appveyor.com/project/vidartf/karma-ts-test/build/job/nbrb4s84k4ap2nta

@vidartf
Copy link
Contributor Author

vidartf commented Dec 8, 2016

I saw c4b9571 now, and following the comments were able to load the JSON with the preprocessor. Unfortunately, it seems like the preprocessor adds a field default to the loaded JSON data, which should not be there.

@monounity
Copy link
Owner

I'm removing the stuff I added in c4b9571, it was a bad idea... You'll be able to load the JSON without a special preprocessor, and the extra defaultshould be gone too, the commonjs loader adds a bit of default export magic which JSON content doesn't need, it creates a cyclic dependency. I'm hoping I can commit the fix tomorrow morning!

@vidartf
Copy link
Contributor Author

vidartf commented Dec 8, 2016

Ok. Final request though: It would be nice if it was possible to load JSON data from files whose filename does not have the .json file extension!

@vidartf
Copy link
Contributor Author

vidartf commented Dec 8, 2016

Hmm. Nevermind the last one, as it seems that the normal require function might not be OK with that either.

erikbarke added a commit that referenced this issue Dec 9, 2016
@monounity
Copy link
Owner

Your test project works now with the latest from master, note that I've removed the JSON preprocessor, requiring JSON works automatically now!

@monounity monounity added this to the 2.1.5 milestone Dec 9, 2016
@vidartf
Copy link
Contributor Author

vidartf commented Dec 9, 2016

Thanks! Works well, also in my full project. Eagerly awaiting 2.1.5 👍

@vidartf vidartf closed this as completed Dec 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants