-
Notifications
You must be signed in to change notification settings - Fork 12
add TYPO3 v13 compatibility #26
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
base: master
Are you sure you want to change the base?
Conversation
'imports' => [ | ||
// recursive definition, all *.js files in this folder are import-mapped | ||
// trailing slash is required per importmap-specification | ||
'@typo3/backend/' => 'EXT:backend/Resources/Public/JavaScript/', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The recursive definition here is very problematic - and I don't think it serves any purporse because the EXT:backend JS loading is taken care of by EXT:backend.
If you have any other extension doing overrides of JavaScripts of EXT:backend (e.g. EXT:container) and this extension is loaded after those, the overrides are overwritten again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to have a look into this PR next week or so, along with the others.
Commenting was a good ping and reminder on that.
Path (recursive) is not that of an issue as it is a chain anyway when looked up, but I tend to have "TYPO3 core overrides" in a dedicated sufolder and more specific on the file, explicitly adding the replaced files instead if simply doing that when a file is dropped into that folder, similar like we do that already for examle here:
https://github.com/web-vision/deepl-base/blob/main/Configuration/JavaScriptModules.php
I don't think I take this PR in that way and splitt it up as I prefere dedicated/logical splitted commits / changes. But I will see next week.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my situation, when debugging with Xdebug, the following seemed to happen:
- The configuration from EXT:backend is resolved and applied --> all files from EXT:backend read
- The overrides from EXT:container (single files) are applied
- Then the configuration from the v13 branch of EXT:wv_file_cleanup again included all fields from EXT:backend and the overrides from container were overwritten
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to debug that in core as that is a fancy core behaviour than anyway.
As already mentioned will change that anyway to explicitly add override files explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO it can be just removed as in the v13 branch, no JS from EXT:backend is overridden. Just one JS file included.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will see.
I recently stramlined the toolchain, the next step would be to check the v12 support first (regarding reported issues) and fix that for current main / 2..x line. after that will be branched out and new major than dropping v11 support and adding v13 support. So that needs to wait anyway a little bit.
No description provided.