-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Improved Flow support for Record subclasses #1414
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
Conversation
Not ideal, but this adds dramatically better support for subclassing Records when using Flow. The cost of this is that the default values passed to Record are no longer checked - they should be separately checked :/. I've updated the documentation to show this in the example, as well as including an example of subclassing a Record when using Flow. Fixes #1388
The subclassed example looks great: easy, simple, clear. Just to be sure I understand, the trade-off is that regular Records change from being typed like: type Point3DProps = { x: number, y: number, z: number };
const makePoint3D: RecordFactory<Point3DProps> = Record({ x: 0, y: 0, z: 0 }); to: type Point3DProps = { x: number, y: number, z: number };
const defaultValues: Point3DProps = { x: 0, y: 0, z: 0 };
const makePoint3D: RecordFactory<Point3DProps> = Record(defaultValues); where the default values are defined separately and typed at the point that it is defined? If so, it seems like a reasonable trade-off to me. |
Yep, that's the tradeoff
…On Thu, Oct 19, 2017 at 9:48 PM Andrew Patton ***@***.***> wrote:
The subclassed example looks great: easy, simple, clear. Just to be sure I
understand, the trade-off is that regular Records change from being typed
like:
type Point3DProps = { x: number, y: number, z: number };const makePoint3D: RecordFactory<Point3DProps> = Record({ x: 0, y: 0, z: 0 });
to:
type Point3DProps = { x: number, y: number, z: number };const defaultValues: Point3DProps = { x: 0, y: 0, z: 0 };const makePoint3D: RecordFactory<Point3DProps> = Record(defaultValues);
where the default values are defined separately and typed at the point
that it is defined? If so, it seems like a reasonable trade-off to me.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#1414 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADD0gsSDEMfKnP8r4_PURfpBWvrzz86ks5suCYKgaJpZM4P_4vl>
.
|
How can I use this in my project right now? |
@patriqdesigns Here’s what I’ve been using while we wait for the stable 4.0.0: "immutable": "facebook/immutable-js#npm", This gives you the latest build for npm always. It reflects whatever is in the "immutable": "^4.0.0-rc.9", |
Thanks man! Should have looked into the branches instead of trying to use the specific commit... |
I would love to use this, but am a bit held off because I don't know how close we are to a stable 4.0.0. We don't use immutablejs yet in the current project, and will introduce it, within a limited scope which will be deployed in the coming 4 weeks. Would you recommend I stick to the latest stable version ? |
@alaindresse Do you use flow or typescript? If so, the v4 release candidates are a massive improvement over v3.x. However, there are known issues, and I don’t have any idea about the timeline for stable v4 being released, so if you aren’t using flow or typescript, you should probably start with latest stable and only upgrade once 4.0.0 is released. |
Thanks for the quick reply @acusti! I am indeed using Flow. Where can I see the known issues ? |
You can find them in the open issues that explicitly mention immutable 4. They’re all on the first page of issues, and most of them are about types. But there are also a few about functionality, like #1438 and #1437 (which looks like an undocumented breaking change, but the undocumented part is the rub). Also note that the official docs at http://facebook.github.io/immutable-js/ describe the behavior of the latest release candidate (4.0.0-rc.9 right now), rather than the latest stable version. |
Thanks. I had noticed the point on the official docs, which led me to consider using it for my project. |
* facebook/master: (305 commits) Reduce repetition in COC link `CONTRIBUTING.md` Add CODE_OF_CONDUCT.md fix typo Add link to Code of Conduct in CONTRIBUTING Improved Flow support for Record subclasses (immutable-js#1414) Improve asImmutable() docs (immutable-js#1415) Only include version in require statement in Runkit instances. (immutable-js#1411) Add example for OrderedSet subtract. (immutable-js#1410) Fix code samples in Immutable.d.ts docs (immutable-js#1408) Adds perf tests for `Map.merge`. (immutable-js#1407) Move gulpfile into resources/ Prettier: perf tests Prettier: include resources Prettify: Use es5 trailing commas Merge immutable-js#1403 Document regression test steps. (immutable-js#1405) Demonstrates correct handling of Symbol keys in Map.mergeDeep when the Maps are nested, and initialized with Symbol KV tuples instead of plain JS object literals. (immutable-js#1404) Fix Map#concat (immutable-js#1402) test(ts-definitions): test Immutable.d.ts validity using tsc (immutable-js#1401) 4.0.0-rc.9 ...
* facebook/master: (305 commits) Reduce repetition in COC link `CONTRIBUTING.md` Add CODE_OF_CONDUCT.md fix typo Add link to Code of Conduct in CONTRIBUTING Improved Flow support for Record subclasses (immutable-js#1414) Improve asImmutable() docs (immutable-js#1415) Only include version in require statement in Runkit instances. (immutable-js#1411) Add example for OrderedSet subtract. (immutable-js#1410) Fix code samples in Immutable.d.ts docs (immutable-js#1408) Adds perf tests for `Map.merge`. (immutable-js#1407) Move gulpfile into resources/ Prettier: perf tests Prettier: include resources Prettify: Use es5 trailing commas Merge immutable-js#1403 Document regression test steps. (immutable-js#1405) Demonstrates correct handling of Symbol keys in Map.mergeDeep when the Maps are nested, and initialized with Symbol KV tuples instead of plain JS object literals. (immutable-js#1404) Fix Map#concat (immutable-js#1402) test(ts-definitions): test Immutable.d.ts validity using tsc (immutable-js#1401) 4.0.0-rc.9 ...
Not ideal, but this adds dramatically better support for subclassing Records when using Flow. The cost of this is that the default values passed to Record are no longer checked - they should be separately checked :/. I've updated the documentation to show this in the example, as well as including an example of subclassing a Record when using Flow.
Fixes #1388