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

add import json tests #247

Merged
merged 8 commits into from
Nov 4, 2023
Merged

add import json tests #247

merged 8 commits into from
Nov 4, 2023

Conversation

iambumblehead
Copy link
Owner

re #246

On first implementation, there is an issue. When multiple tests are added around this feature, they fail. Nodejs returns only the first-resolved json result to many callers., even though each mocked-json is requested through its own unique uri.

@iambumblehead
Copy link
Owner Author

the main error blocking this branch is reproduced here https://github.com/iambumblehead/nodejs-import-attributes-repro

@iambumblehead
Copy link
Owner Author

Possibly, if the issue were reported upstream now, it would be de-prioritized and forgotten because of the experimental state of import attributes. Therefore, it might be better to wait and create an issue later when import attributes are more stable.

@iambumblehead
Copy link
Owner Author

This PR enables mocking of imported json files and can hang out here for a few weeks. A commented-out test here fails, because of an acknowledged bug at nodejs upstream.

@koshic @Swivelgames if you are available, please review and give your opinion.

@iambumblehead iambumblehead force-pushed the add-import-json-test branch 3 times, most recently from ce2e7e2 to c5eb9ea Compare October 17, 2023 18:42
@iambumblehead
Copy link
Owner Author

the test only passes in node 21

upstream issue was resolved but maybe the solution has not been ported to node 18 or 20 yet

https://github.com/nodejs/node/issues / 49724

@iambumblehead iambumblehead merged commit 6f2a207 into main Nov 4, 2023
10 checks passed
@iambumblehead iambumblehead deleted the add-import-json-test branch November 4, 2023 02:21
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

Successfully merging this pull request may close these issues.

3 participants