feat: #3322712 Prefer the minified no-conflict Ace build and keep it out of aggregates
Issue: https://www.drupal.org/project/ace_editor/issues/3322712
Alternative to !6 (closed), rerolled onto current 2.0.x. The premise and the measured savings are angrytoast's (credit due); this changes three things their patch got wrong or left out, so it is a separate MR rather than a push to their branch.
1. The preference order was backwards for the most common install. !6 (closed) ranks src-min above src-noconflict. The point of the no-conflict builds is that they do not define the global AMD define/require, which the plain minified build does - and that clashes with any other AMD loader on the page. The npm-asset/ace-builds package (the documented Composer route) ships all four directories, so this ranking is load-bearing on nearly every site. Order is now: src-min-noconflict → src-noconflict → src-min → src.
2. Keeps the Ace assets out of aggregates. Ace derives the directory of its dynamically loaded modes, themes and workers from its own script URL. With JS aggregation on - the Drupal default - ace.js is concatenated into /sites/default/files/js/js_<hash>.js, so every mode-*.js, theme-*.js and worker-*.js fetch 404s, which is also the root cause reported in #3326303. The injected assets now carry preprocess: FALSE and minified: TRUE. Switching build directories alone does not fix this, and a performance MR is the right place for it.
3. Autocomplete extension no longer loads unconditionally. isset($config['auto_complete']) is always TRUE (the key ships in config/install), so ext-language_tools.js loaded on every editor page even with autocomplete off. Now !empty().
Also: the preference logic is a small class (AceEditorLibraryPreference) rather than a _-prefixed global, so it is unit-testable without loading the .module file and survives the hook conversion in !27 (merged) (issue #3594544) with a one-line change; the ace.js pattern is anchored (/ace\.js$/), fixing a latent bug where a parent directory named after the file was mangled; and library_info_alter() now returns early when the library is missing instead of building paths from FALSE.
Tests: AceEditorLibraryPreferenceTest - 7 cases through a data provider, including the src-min versus src-noconflict case that fails today, an unknown custom layout, and the empty-candidate guard. Verified locally: kernel+unit suite green, PHPStan clean, PHPCS clean.
Note on CI: the pipeline copies only src-min-noconflict into libraries/ace, so exactly one candidate exists there and the ranking is never exercised by the functional job. The unit test is the real coverage.
Rerolled onto the merged hook conversion (!27 (merged)): the logic now lives in AceEditorLibraries::libPath() and alterLibraryInfo() instead of ace_editor.module.
AI-Generated: Yes
Checkpoints:
- File an issue
- Addition/Change/Update/Fix
- Testing to ensure no regression
- Automated unit testing coverage
-
Automated functional testing coverage -
UX/UI designer responsibilities - Readability
-
Accessibility - Performance
- Security
- Developer Documentation
-
User Guide Documentation - Reviewed by human
- Code review by maintainers
- Full testing and approval
- Credit contributors
- Review with the product owner
- Release notes snippet
- Release