fix: Test suite now uses config files again #1623
Conversation
|
| if (Object.keys(m).length === 0) { | ||
| try { | ||
| m = require(myConfig); | ||
| } catch {} |
There was a problem hiding this comment.
So this is just making sure that it's actually empty? (and we don't care about an error in that case)
There was a problem hiding this comment.
That was the idea, yeah. By this point we've already ensured the file exists so if it's imported in some capacity and still empty, that's either a file with no exports or whatever this issue is with esm in our test suite. If a user does provide us with an empty config, that shouldn't be an issue.
The try ... catch I believe is unnecessary and I'll remove in a second. Left over fragment that I don't believe will do anything ever.
What kind of change does this PR introduce?
Test suite bug fixes & chores
Did you add tests for your changes?
Yes
Summary
Found in #1622, as it turns out, none of the config files (
preact.config.js) have been loaded in our test suite in a fairly long time. Thecustom-webpacksubject calls a method that doesn't exist anymore and yet the test isn't throwing. The config files aren't even being used.This PR corrects that, though the implementation is a bit rough? Basically if
esmcannot provide the config we simplyrequire()it.The behavior is a bit odd though and I don't quite understand what the issue is.
export default function () {...}worked with our previous set up, butexport default () => {...},module.exports = function () {...}, andmodule.exports = () => {...}did not. With this fix, the twomodule.exportsformats work in the suite again, butexport default () => {...}does not. Not sure why this might be.Does this PR introduce a breaking change?
No