-
-
Notifications
You must be signed in to change notification settings - Fork 826
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
Frontend Framework Rewrite #1821
Comments
Some work has been done on the ds/frontend-framework-rewrite branch. Bootstrap's affix plugin,
Finding alternatives for the rest of the bootstrap plugins has been a bit hard, so no progress has been made on any others yet. Mainly testing + failing. However, we might be able to use some of them with Zepto (some patches needed, but not hard). Haven't touched Mithril v2 or TypeScript yet (excluding some testing done with both previously). The built dist files have gone down to 268K (forum) and 188K (admin) from 356K and 276K respectively. |
Why use some outdated libraries such as: zepto, jump.js, so the library has not been updated for a long time. |
I'm not convinced about Zepto either. What's the reason behind that choice? Is it only for the reduced size of the library? I'm not seeing any other advantage, as it seems it hasn't been updated for 3 years, and others have dropped it for performance reasons. Plus I think it could create issues with extensions that rely on jQuery being available. |
In a best case scenario, we wouldn't need jQuery at all to do what we want. However, as we probably don't want to reinvent the wheel, we use plugins that chose jQuery for easier coding instead of native implementations. By switching to Zepto, we can minimize the impact jQuery has on the bundle size and maintain compatibility with some jQuery plugins with some patches to Zepto (see js/src/common/utils/patchZepto.js). I don't think performance is much of a concern here as jQuery is only used when necessary (and could probably be removed completely with some work). And we don't really take advantage of many of the features jQuery has to offer, most of which can be done better natively (http://youmightnotneedjquery.com/), apart from IE 11 support. Extensions that rely on jQuery being available will still probably work, unless they use complex selectors not supported by Zepto. Besides, this is a rewrite, and while I am trying to make everything as backwards-compatible as possible, there will be breaking changes. We are on beta versions after all 🤷♂. And a library doesn't necessarily have to be recent to work well. The issues that Hopefully that answered some of your questions 🙂. |
I am looking forward to updating to mithril.js v2, but I don't know what I can do. |
Do you mean to help out with the development, or in terms of extensions ? This isn't a priority, so if you do want to help with Flarum development, please do look at issues in the beta 10 milestone 🙂. |
Some more work has been done.
Build dist files have gone up by 20K each, now 288K (forum) and 208K (admin). |
I have started on Mithril v2 and Typescript. The best way to do this, after a few failures, is to start from scratch and bring in existing JS files one by one - i.e. convert files to typescript & mithril v2 as they are brought in. Thanks to @clarkwinkelmann for his help during this as well. We'll probably want to make our Component class as close to Mithril's components, so those docs can be followed. However, we have introduced a few improvements such as:
Some different things that we've decided not to make backwards compatible / keep the old structure for:
On the side of TypeScript, I've tried to make it so developers can prevent errors by making component props a type (optional). Sadly, it doesn't work with The types & interfaces do not take up any compiled dist space (they appear as empty lines) so that's great 🙂. export type ComponentProps = {
children?: Mithril.Children,
className?: string;
}
export default class Component<T extends ComponentProps = any> {
protected props = <T> {};
} export interface ButtonProps extends ComponentProps {
title?: string,
type?: string,
icon?: string,
loading?: boolean,
disabled?: boolean,
onclick?: Function
}
export default class Button<T extends ButtonProps = ButtonProps> extends Component<T> {} |
I have pushed the code I have so far to ds/frontend-framework-rewrite-mithril and flarum-webpack-config@ds/frontend-framework-rewrite. There's an I have added a markdown file to the root of the branch that contains some of the changes that have been made to the class structures & methods (exlcluding mithril v2 and typescript). Haven't set up typings yet for flarum/core (and definitely not extensions 🙈), including thinking of how they will be generated, published, etc... and the global shims are a bit of a mess, at least with PhpStorm not liking them but recognizing them (???, I don't know TypeScript typings don't do this to me) This is basically what I have right now. I'll probably configure Dist files are currently 196K (forum) and 132K (admin). Of course, it barely includes any of the classes necessary for Flarum to function like a forum. 😁 |
Progress on the Mithril rewrite, excluding components / utils / helpers that still have to be copied over. Outdated checklist removed. See OP for recent updates. /Franz |
As per today's kickoff meeting, here's the proposed roadmap for the remainder of this rewrite. The goal here is to have maximally modular PRs, so that history is as clean as possible. Plan is as follows: Outdated checklist removed. See OP for recent updates. /Franz |
A possible animation library replacement would be http://velocityjs.org/ |
And instead of Zepto, |
See #2255 for Mithril 2 updates |
Consider replacing classList with https://www.npmjs.com/package/classnames? |
Can someone distill every todo from this issue (and other issues) for the rewrite into the OP of this issue and include this checklist to signify rewriting bundled extension progress:
|
What I did:
|
I've started ticking the boxes for the extensions where the Mithril 2.0 upgrades are ready to be merged. Note that we will only merge these once the upgrade in core has been merged. |
- Update libraries - Add Typescript typings for Mithril - Rename "props" to "attrs" - New lifecycle hooks - Other mechanical changes, following the upgrade guide - Remove some of the custom stuff in our Component base class - Introduce "fragments" for non-components that control their own DOM - Remove Mithril patches, introduce a few new ones Challenges: - Behavior of links to current page changed in Mithril - Native Promise rejections are shown on console when not handled - ... Refs #1821. Co-authored-by: Alexander Skvortsov <sasha.skvortsov109@gmail.com> Co-authored-by: Matthew Kilgore <tankerkiller125@gmail.com> Co-authored-by: Franz Liedke <franz@develophp.org>
- Update libraries - Add Typescript typings for Mithril - Rename "props" to "attrs" - New lifecycle hooks - Other mechanical changes, following the upgrade guide - Remove some of the custom stuff in our Component base class - Introduce "fragments" for non-components that control their own DOM - Remove Mithril patches, introduce a few new ones Challenges: - Behavior of links to current page changed in Mithril - Native Promise rejections are shown on console when not handled - ... Refs #1821. Co-authored-by: Alexander Skvortsov <sasha.skvortsov109@gmail.com> Co-authored-by: Matthew Kilgore <tankerkiller125@gmail.com> Co-authored-by: Franz Liedke <franz@develophp.org>
* Update frontend to Mithril 2 - Update Mithril version to v2.0.4 - Add Typescript typings for Mithril - Rename "props" to "attrs"; "initProps" to "initAttrs"; "m.prop" to "m.stream"; "m.withAttr" to "utils/withAttr". - Use Mithril 2's new lifecycle hooks - SubtreeRetainer has been rewritten to be more useful for the new system - Utils for forcing page re-initializations have been added (force attr in links, setRouteWithForcedRefresh util) - Other mechanical changes, following the upgrade guide - Remove some of the custom stuff in our Component base class - Introduce "fragments" for non-components that control their own DOM - Remove Mithril patches, introduce a few new ones (route attrs in <a>; - Redesign AlertManagerState `show` with 3 overloads: `show(children)`, `show(attrs, children)`, `show(componentClass, attrs, children)` - The `affixedSidebar` util has been replaced with an `AffixedSidebar` component Challenges: - `children` and `tag` are now reserved, and can not be used as attr names - Behavior of links to current page changed in Mithril. If moving to a page that is handled by the same component, the page component WILL NOT be re-initialized by default. Additional code to keep track of the current url is needed (See IndexPage, DiscussionPage, and UserPage for examples) - Native Promise rejections are shown on console when not handled - Instances of components can no longer be stored. The state pattern should be used instead. Refs #1821. Co-authored-by: Alexander Skvortsov <sasha.skvortsov109@gmail.com> Co-authored-by: Matthew Kilgore <tankerkiller125@gmail.com> Co-authored-by: Franz Liedke <franz@develophp.org>
app.current
,app.previous
, cleanup Page classes Clean up app.current, app.previous in JS #2156 @askvortsov1app.search
: Remove app.search instance, cache app.cache.searched #2151 @askvortsov1app.cache.discussionList
: Move Discussion List State into its own class #2150 @askvortsov1app.alerts
Extract Alert State, AlertManagerState #2163 @askvortsov1app.modal
Extract modal state #2162 @askvortsov1app.composer
Extract Composer state #2161 @askvortsov1CommentPost
storesPostUser
Don't store PostUser instance in CommentPost #2184 @askvortsov1route
patch, update core and bundled to use a Link component - Use Link component for links instead of mithril route patch #2315console.warn
for BC layer - Add warnings to Mithril 2 BC layer #2313Obsolete Todos / Notes from original approach
See #1950.
@flarum/types
moment
with alternative such asdayjs
jQuery
with alternative such aszepto
jquery.hotkeys
with alternative such asjwerty
mousetrap
bootstrap
, replace with component alternatives where necessarymicromodal
tooltip.js
tooltip.js
html5sortable
withdragula
Important thing to note is to not change too much, we already had a big change with webpack.
The text was updated successfully, but these errors were encountered: