-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Deprecate and simplify some utility functions #9251
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
✅ Deploy Preview for tiddlywiki-previews ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Confirmed: Leilei332 has already signed the Contributor License Agreement (see contributing.md) |
📊 Build Size Comparison:
|
Branch | Size |
---|---|
Base (master) | 2418.3 KB |
PR | 2415.3 KB |
Diff: ⬇️ Decrease: 3.0 KB
Thanks @Leilei332 this is very encouraging, a decent size reduction and much better readability. Do you plan to refactor away the usages of these deprecated methods in this PR? |
I'd love to, but I'm afraid that I will become busy in September. Besides:
Considering these risks, I agree with what Mario said in #7353. IMO the best solution is to rewrite deprecated utility functions to use standard methods, and let developers know they are deprecated and not use them (I think an advantage of #8533 is that it allows a popup to be displayed to show the function is deprecated when a developer is using a deprecated api by using |
@Leilei332 if you have the opportunity, perhaps have a look at TiddlyWiki5/core/modules/utils/dom/browser.js Lines 113 to 159 in 3c45c17
|
|
This reverts commit 650df1d.
This reverts commit 4410ac1.
Hi @Leilei332 this is looking great, thank you. One small thing is that I think it would be helpful to adopt a more distinctive, searchable marker for deprecation comments. Something like:
And then we could introduce further structured comments that use the same prefix. |
Deprecate & simplify some utility functions that are supported in ES2017.