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

[mobx-react-lite] Workaround for typescript issue #43541 #3842

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

r0b1n
Copy link
Contributor

@r0b1n r0b1n commented Mar 7, 2024

There is a TypeScript issue microsoft/TypeScript#43541 that is affecting mobx-react-lite package. This gives some warnings/errors when using mobx-react-lite with Rollup:

node_modules/mobx-react-lite/es/useAsObservableSource.js (1:13) Error: The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten1: var __read = (this && this.__read) || function (o, n) {
                ^
2:     var m = typeof Symbol === "function" && o[Symbol.iterator];
3:     if (!m) return o;

Even though this file/function is not used anywhere in the app it is still included in index.js file and hence parsed by rollup.

In order to work this around I changed the code structure a bit, looks not that great, but this way the output is not polluted. I am not sure if there is a better workaround for this issue.

Old output with the offending __read helper:

var __read = (this && this.__read) || function (o, n) {
    var m = typeof Symbol === "function" && o[Symbol.iterator];
    if (!m) return o;
    var i = m.call(o), r, ar = [], e;
    try {
        while ((n === void 0 || n-- > 0) && !(r = i.next()).done) ar.push(r.value);
    }
    catch (error) { e = { error: error }; }
    finally {
        try {
            if (r && !r.done && (m = i["return"])) m.call(i);
        }
        finally { if (e) throw e.error; }
    }
    return ar;
};
import { useDeprecated } from "./utils/utils";
import { observable, runInAction } from "mobx";
import { useState } from "react";
export function useAsObservableSource(current) {
    if ("production" !== process.env.NODE_ENV)
        useDeprecated("[mobx-react-lite] 'useAsObservableSource' is deprecated, please store the values directly in an observable, for example by using 'useLocalObservable', and sync future updates using 'useEffect' when needed. See the README for examples.");
    var _a = __read(useState(function () { return observable(current, {}, { deep: false }); }), 1), res = _a[0];
    runInAction(function () {
        Object.assign(res, current);
    });
    return res;
}

New output after the change:

import { useDeprecated } from "./utils/utils";
import { observable, runInAction } from "mobx";
import { useState } from "react";
export function useAsObservableSource(current) {
    if ("production" !== process.env.NODE_ENV)
        useDeprecated("[mobx-react-lite] 'useAsObservableSource' is deprecated, please store the values directly in an observable, for example by using 'useLocalObservable', and sync future updates using 'useEffect' when needed. See the README for examples.");
    var res = useState(function () { return observable(current, {}, { deep: false }); })[0];
    runInAction(function () {
        Object.assign(res, current);
    });
    return res;
}

Code change checklist

  • Added/updated unit tests
  • Updated /docs. For new functionality, at least API.md should be updated
  • Verified that there is no significant performance drop (yarn mobx test:performance)

Copy link

changeset-bot bot commented Mar 7, 2024

🦋 Changeset detected

Latest commit: eecf67c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
mobx-react-lite Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@mweststrate
Copy link
Member

mweststrate commented Mar 9, 2024

Thanks for investigating this! Looks great, but would you mind leaving a comment above that line referring to this issue? Otherwise someone in the future might clean this up to write it the old way again.

@r0b1n
Copy link
Contributor Author

r0b1n commented Mar 11, 2024

Thanks @mweststrate ! I added a comment as requested.

@r0b1n r0b1n requested a review from mweststrate March 11, 2024 09:21
@mweststrate
Copy link
Member

LGTM! Thanks!

@mweststrate mweststrate merged commit 7bbb523 into mobxjs:main Mar 28, 2024
1 check passed
@github-actions github-actions bot mentioned this pull request Mar 28, 2024
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.

2 participants