marko-js / marko

A declarative, HTML-based language that makes building web apps fun
https://markojs.com/
MIT License
13.35k stars 643 forks source link

Invalid include target error when setting state in shared component #925

Closed garajo closed 4 years ago

garajo commented 6 years ago

Bug Report

Marko Version: x.x.x

├── marko@4.5.6 
└─┬ marko-starter@1.2.1
  ├─┬ lasso@2.11.21
  │ └── marko@4.5.6  deduped
  └── marko@4.5.6  deduped

Details

I get an error raised in the browser, but I'm not totally sure if the operation I'm doing is correct. I would like to manage state in site-layout, which is a 'shared component with nested attributes'(?), and render value to nav.menu element. Setting state in onMount results in an Invalid include target error.

Adding the following exposes the error src/components/site-layout/index.marko

class {
  onCreate() {
    this.state = { foo: 'bar' }
  }
  onMount() {
    this.setState({foo: 'baz' })
  }
}

Expected Behavior

The component state should be set without erroring

Actual Behavior

Results in an error in the browser. Stack trace below.

Possible Fix

Additional Info ### Your Environment * Environment name and version (e.g. Chrome 39, node.js 5.4): * Operating System and version (desktop or mobile): * Link to your project: Chrome Version 59.0.3071.115 node v8.4.0 Linux Mint 17.3 64-bit ### Steps to Reproduce 1. https://github.com/garajo/marko-Error-Invalid-include-target/commit/a8476d0feeb0626b876b311818ae532b8961b4ce 2. 3. 4. ### Stack Trace ``` Uncaught Error: Error: Invalid include target at EventEmitter.emit (index.js:79) at AsyncVDOMBuilder.emit (AsyncVDOMBuilder.js:309) at AsyncVDOMBuilder.error (AsyncVDOMBuilder.js:216) at doInclude (include-tag.js:22) at includeTag (include-tag.js:30) at wrappedRenderer (helpers.js:106) at render (index.marko.js:85) at renderer (renderer.js:186) at Component.js:469 at Object.batchUpdate [as ___batchUpdate] (update-manager.js:63) emit @ index.js:79 emit @ AsyncVDOMBuilder.js:309 error @ AsyncVDOMBuilder.js:216 doInclude @ include-tag.js:22 includeTag @ include-tag.js:30 wrappedRenderer @ helpers.js:106 render @ index.marko.js:85 renderer @ renderer.js:186 (anonymous) @ Component.js:469 batchUpdate @ update-manager.js:63 ___rerender @ Component.js:458 update @ Component.js:413 updateComponents @ update-manager.js:44 updateUnbatchedComponents @ update-manager.js:16 (anonymous) @ nextTick-browser.js:16 ```

New Feature

Description

Why

Possible Implementation & Open Questions

Is this something you're interested in working on?

DracotMolver commented 6 years ago

Why are you using this.setState() function?. That's from React. As far as I know, you update the state doing this.state.foo = 'baz'

garajo commented 6 years ago

@DracotMolver I got it from the documentation, and it worked with other components. https://markojs.com/docs/components/#methods Am I not understanding this right? Also, it would suck to have to

this.state.foo='bar'
this.state.fi='baz'
etc
DracotMolver commented 6 years ago

@garajo oooh I haven't read that part of the Doc. Thanks for that and sorry, It was my mistake. So, reading the documentation, they this.state.foo = 'baz' ways is the same as using this.replaceState(). Have you tried using the setState() but passing an String instead an Object?.

I'm not sure about this, but this is my theory:

The above sounds ankward, because I could use either setState(name, value) or setState(newState).

Well, after taking a look at the Component.js module, my theory is wrong xd.

setState: function (name, value) {
        var state = this.g_;

        if (typeof name == 'object') {
            // Merge in the new state with the old state
            var newState = name;
            for (var k in newState) {
                if (newState.hasOwnProperty(k)) {
                    state.G_(k, newState[k], true /* ensure:true */);
                }
            }
        } else {
            state.G_(name, value, true /* ensure:true */);
        }
    }

So, yeah, your code should work.

DylanPiercey commented 4 years ago

This issue was likely related to the input.content being server side only and blown away. This has likely been fixed since 4.13.0