Skip to content
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

Clarify a couple of pitfalls I landed in #2806

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

patricksurry
Copy link

Description

Non-functional doc changes that clarify a couple of pitfalls I fell into while learning mithril. Trying to use a class instance as a component rather than the class itself leads weirdness as mithril ends up using the object instance as a prototype for a new object with shadowed properties.

With single keyed fragments I missed the point about keying the child rather than the shared parent object and took a while to realize my mistake. Maybe there's a better explanation for why it's that wya.

Motivation and Context

Save time and simplify the learning curve.

How Has This Been Tested?

Doc.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation change

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • [] I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated docs/changelog.md

Comment on lines +280 to +281
Note the class itself is the component: it's `m(ClassComponent)` not `m(new ClassComponent())`,
with any constructor called in place of the `oninit()` lifecycle method.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is not true. The oninit lifecycle method is still called. This is on rare occasion used with component subclasses, in particular in cases where a supertype needs to do things after the class is fully initialized.

class SuperClass {
    constructor(vnode) {
        // do things to set up superclass
    }

    oninit() {
        // maybe register an instance property or something
    }
}

class SubClass extends SuperClass {
    constructor(vnode) {
        // do things to set up subclass
    }
}

Also, please keep individual paragraphs on one code line to match the surrounding code style. Most editors automatically wrap Markdown, so that shouldn't be much of a problem.

Comment on lines +419 to +420
Note we've added the `key` property to the child of `Layout`,
i.e. it's `m(Layout, m(Person, {key: ...}))` not `m(Layout, {key: ...}, m(Person, ...))`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly is the intent here? I'm not sure how this applies in or out of context. (Also, Latin abbreviations like "i.e." isn't as widely known among non-native speakers, so it's best to avoid those in general in the docs.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants