Fix Typescript 5.6.x support by introducing esnext iterator helpers (#3927) by tonyraoul · Pull Request #3935 · mobxjs/mobx

tonyraoul · 2024-10-08T17:21:23Z

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)

Context

Purpose of the PR

TypeScript 5.6.x introduces new Iterator Helper Methods that enhance the IterableObject interface with methods like map, flatMap, take, and drop. However, when the esnext target is enabled, MobX becomes incompatible as its current implementation lacks these methods, resulting in type errors.

This PR addresses the compatibility issues by:

Updating the IterableObject Interface: Aligning MobX’s implementation with TypeScript’s updated type definitions to prevent type errors.
Integrating Support for Iterator Helper Methods: Adding the new methods within MobX and enabling conditional polyfill support for environments that do not natively implement them.

changeset-bot · 2024-10-08T17:21:27Z

🦋 Changeset detected

Latest commit: 3e17e81

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

This PR includes changesets to release 1 package

Name	Type
mobx	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

tonyraoul · 2024-10-08T17:27:29Z

packages/mobx/src/utils/iterable.ts

+export function makeIterable<T, TReturn = unknown>(
+    iterator: Iterator<T>
+): IteratorObject<T, TReturn> {
+    return Object.assign(Object.create(Iterator.prototype), iterator)


Intentionally avoiding the usage of Iterator.from due to limited availability, using this instead to conditionally add helper functions IIF they are available within runtime either natively or via polyfill.

mweststrate

This looks great, thanks a lot!

zeldigas · 2024-10-16T07:04:23Z

@mweststrate it seems that it is still not fixed. While typecheck is good, I got the following error with typescript 5.6.3 and mobx 6.13.4:

@app/app:test:  FAIL  src/components/organisms/component.store.test.ts > ComponentStore > some test
@app/app:test: ERROR: command finished with error: command (/workspace/master/packages/app) /usr/local/bin/pnpm run test exited (1)
@app/app:test: ReferenceError: Iterator is not defined
@app/app:test:  ❯ makeIterable ../../node_modules/.pnpm/mobx@6.13.4/node_modules/mobx/src/utils/iterable.ts:4:40
@app/app:test:  ❯ makeIterableForSet ../../node_modules/.pnpm/mobx@6.13.4/node_modules/mobx/src/types/observableset.ts:349:12
@app/app:test:  ❯ ObservableSet.values ../../node_modules/.pnpm/mobx@6.13.4/node_modules/mobx/src/types/observableset.ts:237:16
@app/app:test:  ❯ ObservableSet._proto.<computed> ../../node_modules/.pnpm/mobx@6.13.4/node_modules/mobx/src/types/observableset.ts:334:21
@app/app:test:  ❯ _createForOfIteratorHelperLoose ../../node_modules/.pnpm/mobx@6.13.4/node_modules/mobx/dist/mobx.cjs.development.js:308:24
@app/app:test:  ❯ ObservableSet.forEach ../../node_modules/.pnpm/mobx@6.13.4/node_modules/mobx/src/types/observableset.ts:110:9
@app/app:test:  ❯ ComponentStore.call src/components/organisms/component.store.ts:117:31

@app/app:test:     115|   get property(): Abc[] {
@app/app:test:     116|     const array: Abc[] = [];
@app/app:test:     117|     this.uiData.xyz.forEach((selected, item) => {
@app/app:test:        |                               ^
@app/app:test:     118|       if (selected) {
@app/app:test:     119|         array.push(item);

xyz property in code is initialized as xyz: new Set<Abc>() and is located in class that is made autoobservable

tonyraoul · 2024-10-16T07:39:45Z

@app/app:test: ReferenceError: Iterator is not defined @zeldigas
what's your estarget ?

Most likely it's this line
https://github.com/mobxjs/mobx/pull/3935/files#diff-22d6d812427f3e234cc1b5448baf52696bc9a71646c71128e752c61c8a789c31R4

I believe Iterator isn't available to your environment.
FWIW Iterator is not available before es2015, or might be that your setup is missing a lib.

I am a bit unsure about compatibility requirements for mobx, but let me know more about your case maybe I missed something.

zeldigas · 2024-10-16T09:50:54Z

The project I'm working on have specific setup, with mixed use of es5 and es2022 in package.json files. Quick update all the places to es2022 did not help. I'll check in more details later today. Overal of course it's good to provide some isolated example of failure, this is clear..

tonyraoul · 2024-10-16T10:06:32Z

apologies @zeldigas I believe it's not working on safari apparently, thanks for reporting! working on a proper fix.
my apologies for any inconveniences.

tonyraoul · 2024-10-16T10:08:08Z

I believe I misunderstood MDN reference reporting that Iterator.prototype is widely available but that's not the case 🤔

tonyraoul · 2024-10-16T10:49:02Z

my apologies, here is the fix #3943

jansav · 2024-11-11T09:53:00Z

FYI: This PR introduced breaking change by utilizing MapIterator which was introduced in typescript 5.6. It was very annoying to debug why all types break when mobx-version was updated based on semver rules from 6.12.4 -> 6.13.5.

This means that 6.13.x version requires at least typescript 5.6.

thisisanto · 2024-11-15T04:52:17Z

FYI: This PR introduced breaking change by utilizing MapIterator which was introduced in typescript 5.6. It's was very annoying to debug why all types break when mobx-version was updated based on semver rules from 6.12.4 -> 6.13.5.

This means that 6.13.x version requires at least typescript 5.6.

Plus 1 this is a breaking change that's disguised as a patch. Should probably be reverted, and re-introduced along with the appropriate peerDependency bump to typescript@5.6 @tonyraoul

tonyraoul · 2024-11-15T06:34:23Z

apologies for the inconvenience @jansav @thisisanto
I will start working on a fix, if you don't mind I will be adding another patch rather than unpublish this one since I am not a maintainer, hopefully it gets picked up soon.

Thanks for your valuable feedback.

tonyraoul · 2024-11-15T07:59:03Z

Working on a fix here let me know if you have any thoughts please.
For context this happened many times before #3903

It might be a good idea to enable skipLibCheck within your project tsconfig.json

update: I've closed the "Fix" PR for now as it doesn't bring any value vs enabling skipLibCheck, please let me know if you think otherwise.

mweststrate · 2024-11-16T14:11:18Z

@thisisanto @jansav to confirm: we (try to) always release against the latest version of TypeScript (which itself doesn't follow any versioning scheme), and strongly recommend to use skipLibCheck true. For some more details behind the rationale, see #3903 (comment).

tonyraoul added 3 commits October 9, 2024 03:46

Update typescript version

3dc7e5b

Chore(map + set): Add support for esnext Iterator helpers

642fe3f

add changeset

0768528

Update lock file

3e17e81

tonyraoul commented Oct 8, 2024

View reviewed changes

tonyraoul marked this pull request as ready for review October 8, 2024 17:27

tonyraoul changed the title ~~[WIP] Fix Typescript 5.6.x support by introducing esnext iterator helpers (#3927)~~ Fix Typescript 5.6.x support by introducing esnext iterator helpers (#3927) Oct 8, 2024

tonyraoul mentioned this pull request Oct 10, 2024

Typescript 5.6.x compatibility with ObservableSet and Observable Map #3927

Closed

mweststrate approved these changes Oct 15, 2024

View reviewed changes

mweststrate merged commit f91d2e1 into mobxjs:main Oct 15, 2024

github-actions bot mentioned this pull request Oct 15, 2024

Next release #3940

Merged

tonyraoul mentioned this pull request Oct 16, 2024

Fix(mobx): Safer method to consume Iterator.prototype #3943

Merged

3 tasks

tonyraoul mentioned this pull request Nov 15, 2024

Fix(mobx): Typescript pre-5.6 backwards compatibility #3960

Closed

tonyraoul deleted the tonyraoul/mobx-esnext.iterator.helpers-support branch January 19, 2025 11:12

Uh oh!

Conversation

tonyraoul commented Oct 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code change checklist

Context

Purpose of the PR

Uh oh!

changeset-bot bot commented Oct 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

tonyraoul Oct 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mweststrate left a comment

Choose a reason for hiding this comment

Uh oh!

zeldigas commented Oct 16, 2024

Uh oh!

tonyraoul commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zeldigas commented Oct 16, 2024

Uh oh!

tonyraoul commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tonyraoul commented Oct 16, 2024

Uh oh!

tonyraoul commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jansav commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thisisanto commented Nov 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tonyraoul commented Nov 15, 2024

Uh oh!

tonyraoul commented Nov 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mweststrate commented Nov 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tonyraoul commented Oct 8, 2024 •

edited

Loading

changeset-bot bot commented Oct 8, 2024 •

edited

Loading

tonyraoul Oct 8, 2024 •

edited

Loading

tonyraoul commented Oct 16, 2024 •

edited

Loading

tonyraoul commented Oct 16, 2024 •

edited

Loading

tonyraoul commented Oct 16, 2024 •

edited

Loading

jansav commented Nov 11, 2024 •

edited

Loading

thisisanto commented Nov 15, 2024 •

edited

Loading

tonyraoul commented Nov 15, 2024 •

edited

Loading