Added the fetch api #12493

RobinVdBroeck · 2016-11-24T23:22:59Z

Fixes #4948
I didn't know how to write unit tests for it since its actually a library change.

msftclas · 2016-11-24T23:23:04Z

Hi @GamesMaxed, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

Jessidhia · 2016-11-28T07:30:10Z

src/lib/fetch.d.ts

+}
+declare var fetch: typeof window.fetch;
+
+declare type HeadersInit = Headers | string[][] | { [key: string]: string };


Maybe ([string, string])[] instead of string[][]?

I also checked and was slightly disappointed to see it cannot accept arbitrary Iterable<[string, string]> :/

For autocomplete maybe we add list of known headers

type KnownHeaders = 'Accept' | 'Accept-Charset' //...

ctaggart · 2016-12-12T06:25:06Z

I used this for a basic request and blogged about it here:
http://blog.ctaggart.com/2016/12/fetch-api-via-typescript-async.html

Looking forward to seeing this worked on and merged. Thanks for putting this together.

mhegazy · 2016-12-26T18:37:29Z

Gulpfile.ts

@@ -137,13 +137,16 @@ const es2017LibrarySourceMap = es2017LibrarySource.map(function(source) {
 });

 const hostsLibrarySources = ["dom.generated.d.ts", "webworker.importscripts.d.ts", "scripthost.d.ts"];
+const es2015HostLibrarySources = ["dom.iterable.d.ts", "streams.d.ts",  "fetch.d.ts"]


i do not think this should be added to the es6 library. it is not related to the ES6 spec, it is an HTML standard.

How should this be handled? dom.iterable.d.ts would still have to depend on iterables, which will depend on ES6 Symbol, which will depend on ES6 Object...

The file should have /// references on all parts of the es2015 it depends on.

mhegazy · 2016-12-26T18:37:39Z

Jakefile.js

@@ -312,13 +312,16 @@ var es2017LibrarySourceMap = es2017LibrarySource.map(function (source) {
 });

 var hostsLibrarySources = ["dom.generated.d.ts", "webworker.importscripts.d.ts", "scripthost.d.ts"];
+var es2015HostLibrarySources = ["dom.iterable.d.ts", "streams.d.ts",  "fetch.d.ts"]


mhegazy · 2016-12-26T18:38:27Z

src/lib/fetch.d.ts

@@ -0,0 +1,129 @@
+/// The fetch API standard can be found at https://fetch.spec.whatwg.org/
+/// Definitions by: Ryan Graham <https://github.com/ryan-codingintrigue>, Kagami Sascha Rosylight <https://github.com/saschanaz>, Robin Van den Broeck <https://github.com/gamesmaxed>


We have not included any similar headers in the other lib files. please remove that.

mhegazy · 2016-12-26T18:41:43Z

src/lib/fetch.d.ts

+/// The fetch API standard can be found at https://fetch.spec.whatwg.org/
+/// Definitions by: Ryan Graham <https://github.com/ryan-codingintrigue>, Kagami Sascha Rosylight <https://github.com/saschanaz>, Robin Van den Broeck <https://github.com/gamesmaxed>
+
+/// <reference path="lib.streams.d.ts" />


also add a reference to lib.es2015.iterables

and lib.es2015.promise

mhegazy · 2016-12-26T18:42:16Z

src/lib/fetch.d.ts

+}
+declare var fetch: typeof window.fetch;
+
+declare type HeadersInit = Headers | string[][] | { [key: string]: string };


mhegazy · 2016-12-26T18:59:22Z

src/lib/streams.d.ts

+declare class WritableStream {
+    constructor(underlyingSink?: WritableStreamSink, strategy?: QueuingStrategy);
+
+    locked: boolean;


mhegazy · 2016-12-26T18:59:57Z

src/lib/streams.d.ts

+declare class WritableStreamDefaultWriter {
+    constructor(stream: WritableStream);
+
+    closed: Promise<void>;


mhegazy · 2016-12-26T19:00:01Z

src/lib/streams.d.ts

+    constructor(stream: WritableStream);
+
+    closed: Promise<void>;
+    desiredSize: number;


mhegazy · 2016-12-26T19:00:05Z

src/lib/streams.d.ts

+
+    closed: Promise<void>;
+    desiredSize: number;
+    ready: Promise<void>;


mhegazy · 2016-12-26T19:02:15Z

Gulpfile.ts

@@ -153,7 +156,7 @@ const librarySourceMap = [

    // JavaScript + all host library
    { target: "lib.d.ts", sources: ["header.d.ts", "es5.d.ts"].concat(hostsLibrarySources) },
-    { target: "lib.es6.d.ts", sources: ["header.d.ts", "es5.d.ts"].concat(es2015LibrarySources, hostsLibrarySources, "dom.iterable.d.ts") }
+    { target: "lib.es6.d.ts", sources: ["header.d.ts", "es5.d.ts"].concat(es2015LibrarySources, hostsLibrarySources, es2015HostLibrarySources) }


you also need to add entries in commandlineparser.ts for the two new lib targets.

prabirshrestha · 2016-12-29T17:26:53Z

src/lib/fetch.d.ts

+
+/// <reference path="lib.streams.d.ts" />
+
+interface Window {


I have always been curious why fetch has been under window - window.fetch(). Why not just make it so that fetch() works out of the box? Or might be could support both?

@prabirshrestha also works without window as #12493 (diff) has declare var fetch: typeof window.fetch; 🌹

Yup, fetch should be available in non window contexts like Service Workers.

RyanLamansky · 2017-02-09T01:44:16Z

Has this PR been abandoned? I'd love to see this get merged in after the concerns are addressed...

RobinVdBroeck · 2017-02-09T13:29:26Z

Sorry, I was busy in real life with exams and some private stuff and I totally forgot about this. What needs to be done now? Since this is one of my first PRs I'm not familiar with how the changes work? Do I need to pull them into my repo? What can I do now?

remojansen · 2017-02-09T13:51:45Z

@GamesMaxed just update your fork based on the comments and push to your fork again. The new commit will be built by CI. If the build has no errors and all the changes have been done they will merge your PR.

mhegazy · 2017-02-09T19:49:45Z

The fetch API should be in the standard library after #13856

Added the fetch api

1a64ba7

msftclas added the cla-signed label Nov 24, 2016

Fixed some spacing issues

e9e4821

Jessidhia reviewed Nov 28, 2016

View reviewed changes

mhegazy suggested changes Dec 26, 2016

View reviewed changes

prabirshrestha reviewed Dec 29, 2016

View reviewed changes

basarat mentioned this pull request Dec 31, 2016

Fetch API definitions #4948

Closed

mhegazy closed this Feb 9, 2017

microsoft locked and limited conversation to collaborators Jun 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added the fetch api #12493

Added the fetch api #12493

RobinVdBroeck commented Nov 24, 2016

msftclas commented Nov 24, 2016

Jessidhia Nov 28, 2016

mhegazy Dec 26, 2016

mohsen1 Jan 1, 2017 •

edited

Loading

ctaggart commented Dec 12, 2016 •

edited

Loading

mhegazy Dec 26, 2016

Jessidhia Dec 27, 2016

mhegazy Dec 27, 2016

mhegazy Dec 26, 2016

mhegazy Dec 26, 2016

mhegazy Dec 26, 2016

mhegazy Dec 26, 2016

mhegazy Dec 26, 2016

mhegazy Dec 26, 2016

mhegazy Dec 26, 2016

mhegazy Dec 26, 2016

mhegazy Dec 26, 2016

mhegazy Dec 26, 2016

prabirshrestha Dec 29, 2016

basarat Dec 30, 2016

mohsen1 Jan 1, 2017

RyanLamansky commented Feb 9, 2017

RobinVdBroeck commented Feb 9, 2017 •

edited

Loading

remojansen commented Feb 9, 2017

mhegazy commented Feb 9, 2017

		@@ -0,0 +1,129 @@
		/// The fetch API standard can be found at https://fetch.spec.whatwg.org/
		/// Definitions by: Ryan Graham <https://github.com/ryan-codingintrigue>, Kagami Sascha Rosylight <https://github.com/saschanaz>, Robin Van den Broeck <https://github.com/gamesmaxed>

Added the fetch api #12493

Added the fetch api #12493

Conversation

RobinVdBroeck commented Nov 24, 2016

msftclas commented Nov 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohsen1 Jan 1, 2017 • edited Loading

Choose a reason for hiding this comment

ctaggart commented Dec 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RyanLamansky commented Feb 9, 2017

RobinVdBroeck commented Feb 9, 2017 • edited Loading

remojansen commented Feb 9, 2017

mhegazy commented Feb 9, 2017

mohsen1 Jan 1, 2017 •

edited

Loading

ctaggart commented Dec 12, 2016 •

edited

Loading

RobinVdBroeck commented Feb 9, 2017 •

edited

Loading