Fix .js.flow definitions
#829
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The one in |
|
@FezVrasta Yes, but it would be a "breaking" I guess, after changes this file will not be usable as "library definition" anymore. But probably not much users try to use it as such? |
|
It was never intended to be used as library definition, I just wanted it to work as is out of the box, so I'd say it falls in the "breaking change of something that never intended to work that way", which is good to me if you ask me. |
- `.js.flow` files shipped alongside the main entry point are different from library definitions, e.g.: flow-typed. - They need an `@flow` pragma - They can't be wrapped in a `declare module` block - They need to export their public types
|
@FezVrasta I pushed the updated file. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
in the current state of affairs, the
umd/popper.js.flowdefinitions shipped alongside theumd/popper.js(package main entry) just doesn't work. It all resolves toany.That's why you have to do this sort of thing which should not be necessary.
Causes:
.js.flowdeclaration files have a different behaviour and syntax than "library definitions".// @flowpragmadeclare moduleswrapperexport type ...So I managed to make this file work locally by applying all the points above, but I'm not sure how would you like to integrate it in the build workflow here. Because it makes this file significantly different from the
index.js.flowat the package root. So waiting for feedback before pushing something.My advice would be, don't maintain two files. Either only include the
popper.js.flow(modified file).Or move the library definition to
flow-typed.