Skip to content
This repository was archived by the owner on Mar 3, 2023. It is now read-only.

Conversation

@UziTech
Copy link
Contributor

@UziTech UziTech commented Nov 6, 2020

Requirements for Contributing a Bug Fix

Identify the Bug

If the atomTestRunner is transpiled by typescript or anything else using atomTranspilers into an ES module the test runner will not run.

https://github.com/UziTech/terminal/blob/a116a214b6690390dfa656f61813c0596ba7523e/package.json#L30-L36

Description of the Change

set testRunner to testRunner.default if testRunner.__esModule is set to true;

Alternate Designs

Possible Drawbacks

none

Verification Process

compiled Atom and with this fix and tested the code at https://github.com/UziTech/terminal/tree/custom-runner-ts

Release Notes

Allow atomTestRunner to point to an ES Module

@aminya
Copy link
Contributor

aminya commented Nov 6, 2020

A similar patch is applied for the entry point of the packages in #21112.

@UziTech
Copy link
Contributor Author

UziTech commented Nov 7, 2020

That PR seems a lot more robust than this one. Do we want to make that a util function somewhere so we can use it in more places?

@aminya
Copy link
Contributor

aminya commented Nov 7, 2020

Sure, I can do that. Do tastRunners have "activate" function too? I did an extra check for the "activate" function in that PR (I assumed all packages have "activate" just for extra safety😄). If that's not the case. I will drop that extra check.

@UziTech
Copy link
Contributor Author

UziTech commented Nov 7, 2020

No testRunner should just be a function.

@UziTech
Copy link
Contributor Author

UziTech commented Nov 7, 2020

You only need to test for __esModule === true. The only reason for a module to have that property is if it wants to be treated like an es module.

@aminya
Copy link
Contributor

aminya commented Nov 7, 2020

I added the function.

Here is how to use it: https://github.com/atom/atom/pull/21112/files#diff-a1e5876f027cf761e7ec01e3a21fd32319f0d64d015915e4595add656077aa7fR11

You should change the target of your PR to my branch I think.

@sadick254
Copy link
Contributor

Closing this in favour of #21112

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants