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

No longer rely on JSDoc types alone #725

Closed
tmcw opened this issue Apr 13, 2017 · 3 comments
Labels

Comments

@tmcw
Copy link
Member

@tmcw tmcw commented Apr 13, 2017

Right now documentation.js transforms Flow types to JSDoc types, so that it can run them both through format_type, which is derived from doctrine's formatter. We shouldn't do this anymore: JSDoc types can't cover everything that Flow can.

Possibly we should reverse this process and instead convert JSDoc types to Flow types. We will need to survey what the overlap and intersection of the two are in order to make an informed decision.

JSDoc Flow notes
union yes, string|number yes, string|number https://flow.org/en/docs/types/unions/
intersection no yes https://flow.org/en/docs/types/intersections/ eslint/doctrine#183
nullable yes, ?string yes, ?string https://flow.org/en/docs/types/maybe/
arrays of types yes, Array<string> or string[] or Array.<string> yes, Array<string> or string[] https://flow.org/en/docs/types/arrays/
non-nullable yes, !string no?
intersection no yes, type & type https://flow.org/en/docs/types/intersections/
tuple no yes, [string, number] https://flow.org/en/docs/types/tuples/ google/closure-compiler#379
literal types no yes, 2 https://flow.org/en/docs/types/literals/
@tmcw tmcw added the refactor label Apr 13, 2017
@tmcw

This comment has been minimized.

Copy link
Member Author

@tmcw tmcw commented Apr 24, 2017

Another JSDoc lacking: indexers.

http://usejsdoc.org/tags-type.html

versus

https://flow.org/en/docs/types/objects/#object-type-syntax-a-classtoc-idtoc-object-type-syntax-hreftoc-object-type-syntaxa

There is no way to express

{
  size: number,
  [id: number]: string
}

In JSDoc - you can have a type application or a record type, but not both.

@faassen

This comment has been minimized.

Copy link

@faassen faassen commented Jun 8, 2017

non-nullable in flow seems to be simply the variable without ? in front of it. So with flow you mark nullability: ?string

@tmcw

This comment has been minimized.

Copy link
Member Author

@tmcw tmcw commented Oct 13, 2017

Punting, moving conversation to dx-spec tmcw/dx-spec#2

@tmcw tmcw closed this Oct 13, 2017
mollymerp pushed a commit to mapbox/mapbox-gl-js that referenced this issue Aug 20, 2018
Molly Lloyd
Intersection types are not yet supported by documentation.js
documentationjs/documentation#725 (comment)
documentationjs/documentation#694
@mollymerp mollymerp mentioned this issue Aug 20, 2018
2 of 2 tasks complete
mollymerp added a commit to mapbox/mapbox-gl-js that referenced this issue Aug 21, 2018
m1kak added a commit to localizedev/mapbox-gl-js that referenced this issue Sep 19, 2018
* v0.48.0-beta.1 (mapbox#7102)

* Use absolute asset URL in hover-styles example

* android-v6.4.0, ios-v4.4.0, macos-v0.10.0

Updated the style specification to note native support for symbol-placement: line-center.

* Don't reset feature state

* v0.48.0 (mapbox#7144)

* Adds example demonstrating the format expression  (mapbox#7151)

* adds an example utilizing the format expression

* fix indentation

* fix tags

* upate style to streets-10

* add country-label-md and country-label-sm

* fix lint error

* fix docs typo

* document that sprite and glyphs style-spec properties must be absolute

* fix documentation for Map#fitBounds

Intersection types are not yet supported by documentation.js
documentationjs/documentation#725 (comment)
documentationjs/documentation#694

* bump documentation.js

* edit title of format expression example (mapbox#7164)

* update WMS url to include transparent param (mapbox#7189)

* add marker from a place name using forward geocoder api example (mapbox#7152)

* Adds elm-mapbox integration (mapbox#7210)

This is an Elm wrapper library.

* Exponential interpolation example (mapbox#7208)

* add example utilizing eponential interpolation

* remove comment

* fix type

* remove useless br tag

* adjust top margin

* small styling and naming changes

* add cubic bezier example (mapbox#7215)

* add cubic bezier example

* changed interpolation ramp and comments

* typo

* Fix flyTo when the final zoom value is not the requested one (mapbox#7222) (mapbox#7223)

* Fix flyTo when final zoom is not requested one (mapbox#7222)

This complete the fix for mapbox#6828 when the final zoom value is
different from the zoom requested.

* Fix final center position and add test case.

* Mark uuid and validateUuid @Private

* Add symbol-z-order property to symbol layout style spec

* sync-mb-pages

* turn off devBrowserList to include support for IE11

* Remove deprecated benchmark scripts from yarn start

* Add regression test for gl-native issue #12812.

* Mark allow-overlap symbols visible even outside of collision grid.
Fixes issue mapbox#7172.

* implement custom style layers (mapbox#7039)

* Update yarn install instructions for Linux in CONTRIBUTING.md

Since Ubuntu version 18.04 (and possibly as early as 17.04), the package
cmdtest has a "Provides: yarn" entry, so it gets installed when one runs
`sudo apt-get install yarn`. It also doesn't seen Ubuntu ever had a
package for yarn, so add instructions to install it from the right
sources.

* update platform support tables for *-pattern (mapbox#7232)

* make sure there are no zero-area textures

* move potpack to dependencies

* update rollup resolve/commonjs plugins (mapbox#7257)

* Enable face culling for fill-extrusion layers. (mapbox#7178)

* to-color should be idempotent

* Update Benchmarks readme (mapbox#7262)

Update Benchmarks readme to include access token in command for running style benchmarks.

* ["to-array", <item type>, <empty array>] should work for any item type

* Add an explicit test for array assertion fallback behavior

This is tested implicitly via legacy function conversion tests added in mapbox#7095, but it's proving difficult to get those passing on native, and we should test this in isolation anyway.

* Make "to-number" of null behave as documented

* fix missing repeats of CanvasSource when it crosses the antimeridian (mapbox#7273)

fix mapbox#7271

* v0.49.0 changelog (mapbox#7278)

* make sure the correct style is used (mapbox#7277)

* add test for invalid pattern (mapbox#7286)

* Increase fill_extrusion base/height precision.
Fixes issue mapbox#7247: heights over 65,536 meters don't render on some devices.
See discussion in issue mapbox#2096.

* Auto-convert concat arguments to strings

* For string-valued properties, do coercion rather than assertion

* Simplify how token strings are converted

No longer need to-string when using concat. However, it is still needed if the token string consists of a single token.

* Auto-convert format arguments to strings

* Fix implicit conversions for "formatted" type

* Split Collator/Formatted types into separate files

This avoids a troublesome circular import.

* Complete transition to coercing 'text-field' to 'formatted' at evaluation time.
 - Add coercion after call to 'getValueAndResolveTokens`, since the non-expression pathway here skips the coercion logic in parsing_context
 - Remove 'string | Formatted' typing and 'text instanceof Formatted' checks
 - Add Coercion support for 'Formatted', along with dedicated serialization
 - Use Coercion when parsing expected.kind === 'formatted' instead of directly creating a FormatExpression. This is necessary to accommodate expressions such as 'coalesce' that introduce a typeAnnotation.

* Add expression test cases for implicit format type annotations.

* add cluster mapreduce feature to style spec

* add map-reduce options for clustering
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.