openapi-framework: Fix dynamic import to work with jest (fixes #909) #908
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When using jest this code gets transpiled to `require` and the file URL syntax of the import is not supported and breaks the testing environment. This change only uses file URL import syntax on windows machines. This fixes our tests and is still supported on linux and mac operating systems. I think windows is definitely in the minority here and we should not optimize for it. Furthermore, I believe using a UNC path would be a better solution outright but I cannot test because I don't have a window machine at hand. Why not just use jest's `--experimental-vm-modules` flag? Glad you asked. Because of this [jest bug](jestjs/jest#11434 (comment)) which is caused by this [node bug](nodejs/node#37648) which is ultimately caused by this [v8 bug](https://issues.chromium.org/issues/40784051)
coryasilva
changed the title
coryasilva
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When using jest this code gets transpiled to
requireand the file URL syntax of the import is not supported and breaks the testing environment. This change only uses file URL import syntax on windows machines. This fixes our tests and is still supported on linux and mac operating systems.I think windows is definitely in the minority here and we should not optimize for it. Furthermore, I believe using a UNC path would be a better solution outright but I cannot test because I don't have a window machine at hand.
Why not just use jest's
--experimental-vm-modulesflag? Glad you asked. Because of this jest bug which is caused by this node bug which is ultimately caused by this v8 bugNote: This checklist isn't meant to show up on the actual Pull Request (PR). It is added here to make you aware of items our maintainers will look for when reviewing your PR. If your PR is missing any of these items it will be rejected! Please delete this message and the following checklist and replace it with your own message as you see fit.
express-openapi: fixing something) Note: You can use the bin/commit script to automate this.openapi-jsonschema-parameters: Adding examples to the validation keywords (fixes #455)).