Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
In #3255504: Remove jQuery dependency from date.js, @alexpott wrote:
Looking at #3202818: Remove untrue declaration of Opera Mini (in reverse proxy mode) support from Drupal 9.3 we don't support Opera Mini - so I think we can file a follow-up issue to completely remove the library in Drupal 10.0.x and deprecate it in Drupal 9.4.x
See that issue for more details. Deprecate the library.
API changes
core/drupal.date
is deprecated. No replacement is needed.
Release notes snippet
JavaScript library
core/drupal.date
is deprecated. No replacement is needed because no supported browsers require it.
Comment | File | Size | Author |
---|---|---|---|
#30 | 3256549-30-D10.patch | 10.62 KB | murilohp |
| |||
#28 | interdiff_27-28.txt | 1.09 KB | murilohp |
#28 | 3256549-28.patch | 3.77 KB | murilohp |
#27 | interdiff_25-27.txt | 1.73 KB | murilohp |
#27 | 3256549-27.patch | 3.77 KB | murilohp |
Comments
Comment #2
cilefen CreditAttribution: cilefen commentedComment #4
cilefen CreditAttribution: cilefen commentedThere is a usage in in Drupal\Core\Render\Element\Date. I am not sure if removing the accompanying data-drupal-date-format DOM attribute would be part of this.
Comment #5
cilefen CreditAttribution: cilefen commentedHere it is with the usage removed.
Comment #6
hmendes CreditAttribution: hmendes at CI&T commentedChanging this to RTBC as it deprecated the library.
Note: Looks like the usage in Drupal\Core\Render\Element\Date was actually introduced together with the drupal.date library in #1835016: Polyfill date input type
Comment #7
catchI think we should be removing the supporting markup here too.
Comment #8
cilefen CreditAttribution: cilefen commentedI think we’ll need a change record also, yes?
Comment #9
cilefen CreditAttribution: cilefen commentedComment #10
cilefen CreditAttribution: cilefen commentedComment #11
cilefen CreditAttribution: cilefen commentedThe pandemic has been hard on everyone... I think I fixed all my typos and I'm actually following the proper format now.
Comment #13
cilefen CreditAttribution: cilefen commentedRemoved a callback usage.
Comment #14
SpokjeLooking good, just 2 remarks:
Since we removed the usage of this method in
getInfo()
, I believe the proper way of deprecating this is by simply adding the@trigger_error
as the first line and leave the rest intact.processDate()
is triggered. Since there's no existing test of this method, I believe a new Class calling this method is needed with a method callingprocessDate()
and the appropriate$this->expectDeprecation()
just before that line.Covid related test-failures are the hardest ones ;)
Comment #15
cilefen CreditAttribution: cilefen commentedI will be surprised if this passes every coding standard test.
Comment #16
SpokjeTestBot agrees with the above :(
Comment #17
cilefen CreditAttribution: cilefen commentedAs expected...
Comment #18
alexpottDon't we still need to add the library in Drupal 9 - to maintain support there? I agree we can remove the library in Drupal 10 but I think this might be once of those instances where deprecation doesn't really work. Or is the case the even in Drupal 9.4 there are no supported browsers that need this library? I thought ie support is only being dropped in Drupal 10.
Comment #19
cilefen CreditAttribution: cilefen commentedI think no. #3202818: Remove untrue declaration of Opera Mini (in reverse proxy mode) support from Drupal 9.3 seems just to have removed Opera-mini compatibility in 9 rather than deprecating anything.
Comment #20
cilefen CreditAttribution: cilefen commentedI used to like all this tooling.
Comment #21
cilefen CreditAttribution: cilefen commentedI have no idea on that point.
Comment #22
cilefen CreditAttribution: cilefen commentedhttps://caniuse.com/input-datetime
Comment #23
cilefen CreditAttribution: cilefen commentedSo IE needs it and yes, there really isn't a way to deprecate this. I suppose it should simply be removed in 10?
Comment #24
catchWe could deprecate it, but then suppress the deprecation notice in the listener, then it would be documented but wouldn't actually trigger deprecation notices. We never do that for PHP deprecations any more (or try not to at least), but maybe for a library like this it'd be fine.
Comment #25
longwaveSo we have to leave ::processDate in place for now, let's see how many tests this blows up.
Comment #27
murilohp CreditAttribution: murilohp at CI&T commentedJust following your suggestion here @catch, let's see the tests now.
Comment #28
murilohp CreditAttribution: murilohp at CI&T commentedFixing CS here
Comment #29
catch#28 looks OK to me, the only other option with ::processDate() would be to mark it @internal, for removal in Drupal 10, but I don't think it hurts to have @deprecated on there and suppress it.
We need a 10.x patch with the usage/library etc. all removed too.
Comment #30
murilohp CreditAttribution: murilohp at CI&T commentedGreat! Here's a D10 only patch.
Comment #31
murilohp CreditAttribution: murilohp at CI&T commentedThinking about it here, I think we need to merge #28 on D10 first @catch, this way for D10 patch, I can remove the deprecated test class and change the deprecation listener trait, what do you think?
Comment #32
catch@murilohp it's usually easier to have completely separate Drupal 10 and Drupal 9 patches, which can be committed at the same time (in the window where we're still committing deprecations to Drupal 9 and removing them from Drupal 10, otherwise we have to follow the workflow you suggested). In which case #28 and #30 both look good to me.
Actually let's mark this RTBC.
Comment #33
alexpottCommitted 2fc40e8 and pushed to 10.0.x. Thanks!
Committed baa0ed6 and pushed to 9.4.x. Thanks!