-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Flow errors with flow 0.90.0 and draft-js-js 0.10.5 #1974
Comments
Gotcha. This will be fixed on the next release. Thanks for pointing it out! 👍 |
Thanks! do you have an idea when that release will be out? |
We're currently working on cutting down the size of the release, which is the main blocker for a new release. If our efforts are successful, should be fairly soon :) cc @claudiopro who is working on this |
Any news? Because flow doesn't work correctly without |
This will be fixed in the release of 0.11. We're in a bit of a difficult situation here, since it's been a while since we an official release is made, so we wan't to minimize the risk of releasing. There's two main fronts:
Last I checked, we found a couple of issues on the latter. I'll investigate further into mitigating that issue as well as we can and getting 0.11 out the door sooner rather than later— I'm personally just ramping into some busy times in my life (big move, changing cities) so I can't say I'll have much time to commit to this myself until May. Updates around the release will probably come in #1312 though. |
I'm still getting Flow errors in the Draft.js code with the v0.11.0-beta2 (& Flow v0.91.0), consistently across the 3 projects where I use Draft.js and Flow. Is there something I'm missing? Here are the errors for reference: Flow errors
I think I was getting all of those errors before, except for |
These flow errors seem to result from a newline being inserted after the |
Ah, nice catch! Looking further at the specific errors, they all come from the same commit for the Flow v0.68 upgrade a99f51e. I wouldn't be surprised if it was easier to fix them rather than ignore them and change the build pipeline. Property role is missing in DraftEditorProps [1]^ unless I'm missing something, this is just a matter of adding the property to https://github.com/facebook/draft-js/blob/master/src/component/base/DraftEditorProps.js. Property caretRangeFromPoint is missing in Document [1]This one looks slightly trickier. Related Flow issue: facebook/flow#6937. Seems like a potential solution would be to do what's suggested in facebook/flow#396 (comment), declare interface Document extends Document {
// TODO Use the correct signature.
caretRangeFromPoint: () => {},
} Property blockType is missing in ContentBlockConfig [1]^ just need to add the property to the type definition? https://github.com/facebook/draft-js/blob/a99f51eb33dc7c529f21b87d6cc7394b073f9388/src/model/encoding/convertFromHTMLToContentBlocks2.js#L164-L176 If there is some agreement that this is the correct approach here I'm happy to make a PR with the fixes. For the |
Thanks for reporting this issue @yiblet! Let me look into this @thibaudcolas. |
@thread is this still an issue? |
Given the issues with babel, would using example:
becomes
|
@malectro suppression comments are generally preferred. I just ran flow on the repo and see no errors. Closing this issue. |
@mrkev at least for me the problem is with projects I’m using Draft.js and Flow on (#1974 (comment)), not the Draft.js repo itself. Could we leave this issue open on that basis, unless that’s fixed too? Or is there another issue for that? I’m not super up-to-date with this but last time I checked I needed the following overrides to disable Flow checks for Draft.js code, which somewhat defeats the purpose of Draft.js publishing flow type definitions to start with: [ignore]
.*/node_modules/draft-js/lib/DraftEditorContents.react.js.flow
.*/node_modules/draft-js/lib/ContentBlockNode.js.flow
.*/node_modules/draft-js/lib/ContentBlock.js.flow
.*/node_modules/draft-js/lib/DraftEditorLeaf.react.js.flow
.*/node_modules/draft-js/lib/DraftEditorDragHandler.js.flow
.*/node_modules/draft-js/lib/DraftEditor.react.js.flow
.*/node_modules/config-chain
.*/node_modules/fbjs/lib/emptyFunction.js.flow |
@thibaudcolas yeah, let's open a new issue for that. Include the info in both your comments too 👍 |
hey @mrkev i understand suppression comments are preferred, but isn't the issue this?
running Flow on the Draft code base isn't going to yield any errors because the errors only pop up in the built files. if you instead run as an example, |
Oh, I didn't notice this, sorry. Hmm, the build shouldn't be doing this, because comments are the preferred way of suppressing errors. Error codes are coming to Flow soon. FlowFixMe's are inherently unsafe because is something else breaks on a FlowFixMe'd line, Flow won't catch it. Error codes will provide a way to narrow the suppression space and reduce the unsafety of FlowFixMe's, and afaik it will work with comments, not the suppression type. We should look into fixing the build here. |
No problem. Thanks for repoening. This is a bummer because it doesn't look like Babel outputs a consistent position for suppression comments, and this comment implies they have no plans to change things babel/babel#5512 (comment) Without using a suppression type it seems like there are two options:
|
Do you want to request a feature or report a bug?
report a bug
What is the current behavior?
Flow shows 25 different errors when typechecking draft js
If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. You can use this jsfiddle to get started: https://jsfiddle.net/stopachka/m6z0xn4r/.
Here's the output when I run flow in the
lib/
directoryWhat is the expected behavior?
flow shouldn't throw errors
Which versions of Draft.js, and which browser / OS are affected by this issue? Did this work in previous versions of Draft.js?
flow 0.90.0 & draft js 0.10.5
The text was updated successfully, but these errors were encountered: