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

ESM support #445

Closed
wants to merge 8 commits into from
Closed

ESM support #445

wants to merge 8 commits into from

Conversation

giltayar
Copy link
Collaborator

ESM support for testdouble.js, using quibble

@giltayar
Copy link
Collaborator Author

giltayar commented May 13, 2020

Note that it's almost done, but not quite. Currently, to use ESM, you need to node --loader=quibble, but it doesn't make sense to use quibble. It should be node --loader=testdouble. I'll work on that (nothing major, just some glue code, and an "example" test that checks that it all hangs together), but in the meantime, @searls, if you want, you can review the code.

@searls
Copy link
Member

searls commented May 14, 2020

I'm pretty tied up this week, so I'll wait for the all-clear that it's "ready-ready" and then do one big review/fixup en masse if that's ok

@giltayar
Copy link
Collaborator Author

Based on experience, I want to add one more thing: if you didn't specify the loader (node --loader=testdouble) then we should throw an exception to show you that you didn't and not fail silently with unexpected results. For this, I need to change quibble, and then come back here and do a small change.

And then I'm done. Promise! ☺️

@searls
Copy link
Member

searls commented May 15, 2020

I shall await your changes!

@giltayar
Copy link
Collaborator Author

@searls And... we're done! Take your time.

I'll start writing that blog post... :-)

//
// Once these bugs are solved, we can go back to running teenytest regularly for ESM tests

require('../../node_modules/.bin/teenytest')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind setting a todo for yourself a few months from now to come back and revisit this?

@searls
Copy link
Member

searls commented May 16, 2020

Apologies that github is confused about the merge status, but this is merged! And released! Landed in 3.15.0

Great job seeing this through @giltayar!

@searls searls closed this May 16, 2020
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.

None yet

2 participants