microsoft / ReSub

A library for writing React components that automatically manage subscriptions to data sources simply by accessing them
MIT License
607 stars 49 forks source link

Compiler error using @key decorator #98

Closed alariej closed 5 years ago

alariej commented 5 years ago

I have tested the simpler subscription examples successfully, and now I am trying autosubscriptions using the @key decorator, such as in your example:

@autoSubscribe
    getTodosForUser(@key username: string) {
        return this._todosByUser[username];
    }

Unfortunately, I get a compiler error:

Error: Module parse failed: Unexpected character '@' (1:20609) You may need an appropriate loader to handle this file type.

It seems like Babel has a problem with the @key decorator being used within a function, and not with other decorators like @autoSubscribe and @AutoSubscribeStore.

I've checked my babel.config.js and everything seems OK, using:

  const plugins = [
    ['@babel/plugin-proposal-decorators', { legacy: true }],
    ["@babel/plugin-proposal-class-properties", { "loose": true}]
  ];

Did someone else come across this problem? Any recommendation on how to solve this?

deregtd commented 5 years ago

In theory, once the typescript outputs the javascript that babel will be processing, the decorators should be completely gone. Are you trying to use the decorators straight inside javascript?

alariej commented 5 years ago

No, I use it in my typescript code. As I wrote, it is odd, since all decorators other than the @key create no problem at all.

deregtd commented 5 years ago

It appears that using decorators straight in babel has different support than Typescript, and reading the comments, they're actually a little mad that Typescript went ahead and implemented function parameter decorators: https://github.com/babel/proposals/issues/13 (more context at https://github.com/babel/babel/issues/1301)

So I think you have to use Typescript to process the decorators, not babel, at this time.

alariej commented 5 years ago

I see, thanks for info. You mean, I should remove the Babel decorator plugin in babel.config.js? Do I need a Typescript plugin then? Any additional info you can point me to?

deregtd commented 5 years ago

I'm not even sure what the right terminology is, but in most of our apps, we only use babel later in the build pipeline. Typescript compiles the decorators down into non-decorated ES5 or ES6 (depending on the project), and then babel runs some other cleanup on it from there. If you look at this TodoList sample app from the ReactXP repo, it shows using resub with a typescript app and a simplified version of the build pipeline that we use in our private projects:

https://github.com/Microsoft/reactxp/tree/master/samples/TodoList

thegalah commented 5 years ago

Any update on this or on the best way to resolve the issue? None of the examples seem to use the @key function parameter decorator

deregtd commented 5 years ago

Several of the unit tests use @key. You can start with one of those build systems.

BiggA94 commented 5 years ago

UPDATE: Sorry, I used the key function incorrectly. Updated for the working approach :)

I don't know your actual code, but I have the same problem. So my aproach was, that i call the key function manually. Something like

class SubStore extends StoreBase {
@autoSubscribe
    getTodosForUser(username: string) {
        return this._todosByUser[username];
    }
}

key(SubStore .prototype, 'getTodosForUser', 0);

should work.

deregtd commented 5 years ago

Very strange. We have @key working lots of places in our codebase:

    @autoSubscribe
    isLoading(@key username: string): boolean {
    ...
        "resub": "^1.2.0",
        "typescript": "3.5.1",
thegalah commented 5 years ago

Yeah I wasn't able to get @key working. My build config is the same as the create react-xp app from https://github.com/a-tarasyuk/create-rx-app. @BiggA94 's method seems to work though

deregtd commented 5 years ago

@a-tarasyuk Have you tried using @key in any of your samples? I've never actually tried building with the create-rx-app since I always just copy an older project of mine and keep evolving my private stash of garbage. :)

a-tarasyuk commented 5 years ago

@deregtd no, I have not tried. I suppose that the problem with the @babel/plugin-proposal-decorators which doesn't handle parameter decorators., I think there are babel plugins which can be used to add this feature., if we are talking about create-rx-app - it doesn't include all possible babel plugins :).

berickson1 commented 5 years ago

I just ran into this problem with a personal project - using React Native with babel transpiling typescript. As @a-tarasyuk mentioned above, you need an additional plugin to add babel support.

I had success with babel-plugin-parameter-decorator

My plugin list looks like this:

  plugins: [
    ["@babel/plugin-proposal-decorators", { legacy: true }],
    "babel-plugin-parameter-decorator"
  ],
berickson1 commented 5 years ago

I just put out a PR to add a note about babel to our README, this should help unblock consumers using babel. @thegalah, can you try this out?

thegalah commented 5 years ago

I tried adding babel-plugin-parameter-decorator and I get:

Module build failed (from ./node_modules/babel-loader/lib/index.js):
TypeError: Cannot read property 'name' of undefined
berickson1 commented 5 years ago

Opened a bug on babel-plugin-parameter-decorator https://github.com/WarnerHooh/babel-plugin-parameter-decorator/issues/5

WarnerHooh commented 5 years ago

Hi all,

Nice to hear this plugin babel-plugin-parameter-decorator helps. And thanks your feedback @berickson1 . I have published a new version(1.0.8) already. This should fix the problem. Please feel free to let me know if you still have the problem.

berickson1 commented 5 years ago

Thanks @WarnerHooh - this solves my issues. @thegalah - can you bump package version to 1.0.8 and try again?

thegalah commented 5 years ago

@berickson1 I've bumped the package version to 1.0.8 and I now get:

ERROR in ./src/Services/Data/Stores/UserScheduleStore.ts
Module build failed (from ./node_modules/babel-loader/lib/index.js):
TypeError: Cannot read property 'forEach' of undefined
    at node_modules\babel-plugin-parameter-decorator\lib\index.js:84:60
WarnerHooh commented 5 years ago

Hi @thegalah,

Thanks for the feedback, I had a quick fix in 1.0.9. Hope all good if you try with this version.

thegalah commented 5 years ago

This worked for me! Thanks for the great work!