fix regression in ace-builds loading #5218
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #5218 +/- ##
=======================================
Coverage 87.22% 87.22%
=======================================
Files 565 565
Lines 45251 45253 +2
Branches 6927 6927
=======================================
+ Hits 39469 39471 +2
Misses 5782 5782
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
| // backwards compatibility for node and packaged version | ||
| try { | ||
| loadedModule = this.$require(moduleName); | ||
| } catch (e) {} |
There was a problem hiding this comment.
is it good to catch all errors? would it not be better to check whether the method exists or something similar
There was a problem hiding this comment.
require itself may throw errors when moduleName is not loaded yet, this basically restores the old code, before my commit that introduced the regression. I could also move try catch into $require itself, but then i'd have to add that in two places.
fixes https://github.com/ajaxorg/ace-builds/issues/247#issuecomment-1609409571 and e14902e#commitcomment-119729446
Testing is not included, will add puppeteer tests to https://github.com/ajaxorg/ace-samples, checking editor functionality in all configurations to prevent issues like this in the future