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

fix: fixture path to support commonjs and esm #19

Merged
merged 4 commits into from
Apr 8, 2022

Conversation

sewnagP
Copy link
Contributor

@sewnagP sewnagP commented Apr 5, 2022

Added a support for the commonjs and ESM to fixturepath.

Description

Motivation and Context

Screenshots (if appropriate):

Checklist:

  • I have updated/added documentation affected by my changes.
  • I have added tests to cover my changes.

@mlrawlings
Copy link
Member

Hey, thanks for the PR!

I'm happy to support this, however as it stands, this PR changes when require is called (it's now immediately rather than upon usage).

If you wouldn't mind addressing that, I'll merge this once the change is made.


A bit more context:
This desire for lazy evaluation is why fixture is a getter and the require call is inside the render function. Since this library is intended to be used by tests, this means that a fixture with an error will cause a specific test to fail rather than blow up the whole suite. In some cases, you may even want to test that a certain fixture throws. Or if you grep/it.only to run a specific test, the suite is slowed down by unnecessarily loading all the fixtures.

@sewnagP
Copy link
Contributor Author

sewnagP commented Apr 7, 2022

@mlrawlings Thanks, addressed the comments.

@mlrawlings mlrawlings merged commit 28a7081 into marko-js:master Apr 8, 2022
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.

2 participants