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.

Sign up for GitHub

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

Jump to bottom

mobx.decorate doesn't work #1969

Closed
6 tasks done
gykf opened this issue May 19, 2019 · 7 comments
Closed
6 tasks done

mobx.decorate doesn't work #1969

gykf opened this issue May 19, 2019 · 7 comments
Labels
👓 needs investigation

Comments

@gykf
Copy link

gykf commented May 19, 2019

I'm not using any compilers. I'm loading MobX from a CDN. The decorate method simply doesn't seem to work.

I have a:

  1. Issue:
  • Provide error messages including stacktrace
  • Provide a minimal sample reproduction. Create a reproduction based on this sandbox
  • Did you check this issue wasn't filed before?
  • Elaborate on your issue. What behavior did you expect?
  • State the versions of MobX and relevant libraries. Which browser / node / ... version?

Steps to reproduce

The "code sandbox" normally used for reproduction does way more than it should, so you'll have to create a couple of files and spin up a static web server to reproduce this.

  1. Create an HTML file with the following contents:
<html>
  <head></head>
  <body>
    <script src="https://unpkg.com/mobx@5.9.4/lib/mobx.umd.js"></script>
    <script src="mobx-issue.js"></script>
  </body>
</html>
  1. Create mobx-issue.js
class MyClass {
  list = []
}

mobx.decorate(MyClass, { list: mobx.observable })

const instance1 = new MyClass()
console.assert(mobx.isObservable(instance1.list), 'not observable')
console.assert(mobx.isObservableArray(instance1.list), 'not an observable array')

setTimeout(() => {
  const instance2 = new MyClass()
  console.assert(mobx.isObservable(instance2.list), 'not observable')
  console.assert(mobx.isObservableArray(instance2.list), 'not an observable array')
  instance2.list.replace([])
}, 1000)

instance1.list.replace([])

I only added the timeout because it looks like the decorators are added as "pending", but I don't know what that's supposed to mean. The assertions fail regardless.

Expectations

I expect the above assertions in the script to not fail, but they do.

I also expect the instance1.list.replace and instance2.list.replace calls to not throw errors, but they do.

Uncaught TypeError: instance1.list.replace is not a function
    at mobx-issue.js:18
(anonymous) @ mobx-issue.js:18

I suspect the issue is either in the build artifacts, or related to not using Babel/Webpack and other fluff with my source, or some combination of these.

I got as far as understanding that the decorators are applied in some sort of "pending" state. This is MyClass.prototype.Symbol(mobx pending decorators).list:

decoratorArguments: []
decoratorTarget: {constructor: ƒ, Symbol(mobx pending decorators): {…}}
descriptor: undefined
prop: "list"
propertyCreator: ƒ (target, propertyName, descriptor, _decoratorTarget, decoratorArgs)
__proto__: Object

I also know that the decoratorFactory function in MobX (line 322) will get an undefined applyImmediately argument. I'm not sure if this is relevant.

Tested with minified and unminified UMD and "module" builds of MobX version 5.9.4 on Chrome 76.0.3799.0
@urugator
Copy link
Collaborator

urugator commented May 19, 2019 •
edited
Loading

Reproduced the problem - assigned value isn't converted to observable when the "native" (non-transpiled) property initializer is used.

class MyClass {
  a; // note it doesn't matter where the value is assigned  
  constructor() {        
    this.a = [];
    this.b = [];    
  }
}

mobx.decorate(MyClass, { a: mobx.observable, b: mobx.observable })
const i = new MyClass();

console.log(mobx.isObservable(i.a)); // false
console.log(mobx.isObservable(i.b)); // true

@jeremy-coleman
Copy link
Contributor

jeremy-coleman commented Jul 30, 2019 •
edited
Loading

I ran into this issue also, although not using from a cdn. exporting the class immediately fixed my issues. As in, “export class Thing ...” Using a named es export after decorate was the immediate cause, i dont know if that is expected or not, im not sure the es spec knows either

@Bnaya
Copy link
Member

Bnaya commented Nov 27, 2019

class members has defineGetter / defineSetter behaviour that being created per instance
decorate is based on patching the prototype, but the defineGetter is masking the prototype.

seems like decorate has a spec compliancy snag.

@brabeji
Copy link

brabeji commented Jan 8, 2020 •
edited
Loading

For anyone who is trying to test decorate()d class-based observables with ts-jest: Enable basic babel transpilation in ts-jest to overcome this issue.

https://kulshekhar.github.io/ts-jest/user/config/babelConfig

It took me a while to arrive at this so it might save someone some time. Basic .babelrc with just env preset is sufficient.

exporting the class immediately fixed my issues.

@jeremy-coleman this, unfortunately, didn't make any difference in jest

@Kukkimonsuta Kukkimonsuta mentioned this issue Feb 15, 2020
Closed
@danielkcz
Copy link
Contributor

Probably better to focus decorators related discussion to #2325. Closing this one.

@lock
Copy link

lock bot commented Jun 24, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.

1 similar comment
@lock
Copy link

lock bot commented Jun 24, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
👓 needs investigation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@danielkcz @Bnaya @brabeji @gykf @urugator @jeremy-coleman