-
Notifications
You must be signed in to change notification settings - Fork 6k
Typescript-fetch: file upload and Typescript dependency updates #4852
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
Changes from all commits
047debb
89c842b
9c2cb6c
86d7df2
76733ea
2bf37cb
ab2f069
9521400
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,2 @@ | ||
| wwwroot/*.js | ||
| node_modules | ||
| typings |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,16 +6,18 @@ | |
| "browser": "./dist/api.js", | ||
| "typings": "./dist/api.d.ts", | ||
| "dependencies": { | ||
| {{^supportsES6}}"core-js": "^2.4.0", | ||
| {{/supportsES6}}"isomorphic-fetch": "^2.2.1" | ||
| "@types/node": "6.0.46", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be nice to also have this in the angular2 && angular clients.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I started looking at updating angular2 but I'm having issues building the copy in master using either TS 1.8.10 or my installed typescript@next developers build. I'm sure my unfamiliarity with building and using the angular generated libraries isn't helping either! I can look into this but I think it may require more extensive changes and that it's likely better suited for a follow up PR. Is that okay with you?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure. |
||
| "@types/isomorphic-fetch": "0.0.33",{{^supportsES6}} | ||
| "@types/core-js": "0.9.35", | ||
| "core-js": "2.4.1",{{/supportsES6}} | ||
| "isomorphic-fetch": "2.2.1" | ||
| }, | ||
| "scripts" : { | ||
| "prepublish" : "typings install && tsc", | ||
| "prepublish" : "tsc", | ||
| "test": "tslint api.ts" | ||
| }, | ||
| "devDependencies": { | ||
| "tslint": "^3.15.1", | ||
| "typescript": "^1.8.10", | ||
| "typings": "^1.0.4" | ||
| "typescript": "^2.0.7" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,9 +9,6 @@ | |
| }, | ||
| "exclude": [ | ||
| "dist", | ||
| "node_modules", | ||
| "typings/browser", | ||
| "typings/main", | ||
| "typings/main.d.ts" | ||
| "node_modules" | ||
| ] | ||
| } | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,2 @@ | ||
| wwwroot/*.js | ||
| node_modules | ||
| typings |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,9 +9,6 @@ | |
| }, | ||
| "exclude": [ | ||
| "dist", | ||
| "node_modules", | ||
| "typings/browser", | ||
| "typings/main", | ||
| "typings/main.d.ts" | ||
| "node_modules" | ||
| ] | ||
| } | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,2 @@ | ||
| wwwroot/*.js | ||
| node_modules | ||
| typings |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,9 +9,6 @@ | |
| }, | ||
| "exclude": [ | ||
| "dist", | ||
| "node_modules", | ||
| "typings/browser", | ||
| "typings/main", | ||
| "typings/main.d.ts" | ||
| "node_modules" | ||
| ] | ||
| } | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,2 @@ | ||
| wwwroot/*.js | ||
| node_modules | ||
| typings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not instead of this if statement in all the generated code. Use a Mustache if. This way you don't need the next strange code:
maybe do something like this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally used a very similar implementation to the one you suggested and also mentioned in #3921 but noticed that, since
{{#isFile}}is scoped to the individual parameter, it didn't work for requests that included a file and regular form parameters. I checked again and, unless I'm looping through the individualformParams,{{#isFile}}always maps to false.Is there a Mustache template variable available that tracks if any of the included
FormDataattributes is of type file? If so, that would allow me to simplify the mustache template to essentially match your suggestion and get rid of the seemingly extraneous code.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that I'm aware of. If it helps, we can introduce the variable.