Initial code commit #1
Conversation
|
@balena-ci retest |
thgreasi
force-pushed
the
initial-pr
branch
2 times, most recently
from
4f4323e
to
5b49474
Compare
src/index.ts
Outdated
| public backendParams: BackendParams, | ||
| ) { | ||
| super(params); | ||
| if (!backendParams?.fetch && typeof fetch === 'undefined') { |
There was a problem hiding this comment.
It'd be nice to also add a check for typeof backendParams.fetch === 'function'
src/index.ts
Outdated
| typeof body === 'object' | ||
| ? JSON.stringify(body) | ||
| : body == null | ||
| ? null |
There was a problem hiding this comment.
You need to reorder these because
console.log(typeof null) // 'object'but in case you're wondering
console.log(typeof undefined) // 'undefined'
src/index.ts
Outdated
| return fetchImplementation(url, { | ||
| ...options, | ||
| body: normalizedBody, | ||
| }).then((response) => { |
There was a problem hiding this comment.
I'd use async/await for new code
There was a problem hiding this comment.
Let me try whether the transpiled umd build still works on google apps script environment :)
There was a problem hiding this comment.
Looks like it still works 👍
| @@ -9,6 +9,9 @@ | |||
| "sourceMap": true, | |||
| "strictNullChecks": true, | |||
| "target": "es5", | |||
There was a problem hiding this comment.
Any reason we target such an old version?
There was a problem hiding this comment.
The initial reason I made this is to have it running in google sheets where the environment is a bit weird (even though it's V8). For example we can't use const/let there.
There was a problem hiding this comment.
I'd appreciate seeing @balena/es-version used but for now this is ok
tsconfig.json
Outdated
| @@ -9,6 +9,9 @@ | |||
| "sourceMap": true, | |||
| "strictNullChecks": true, | |||
There was a problem hiding this comment.
If possible it'd be nice to make this
| "strictNullChecks": true, | |
| "strict": true, |
as it's all new code so we should be able to conform to fully strict ts
tests/chai.ts
Outdated
| const { expect } = chai; | ||
| export { expect }; |
There was a problem hiding this comment.
| const { expect } = chai; | |
| export { expect }; | |
| export const { expect } = chai; |
src/index.ts
Outdated
|
|
||
| type PromiseObj = Promise<{}>; | ||
|
|
||
| class RequestError extends Error { |
There was a problem hiding this comment.
It'd be nice to export this so downstream can do err instanceof RequestError as necessary to check for the specific error class rather than trying to infer it from the properties - especially since I don't think the typing will be exported currently
package.json
Outdated
| "mocha": "^7.2.0", | ||
| "rollup": "^2.10.0", | ||
| "ts-node": "^8.10.1", | ||
| "typescript": "^3.9.2" |
There was a problem hiding this comment.
| "typescript": "^3.9.2" | |
| "typescript": "^3.9.3" |
package.json
Outdated
| "lint-fix": "balena-lint --typescript --fix src tests", | ||
| "test:node": "mocha -r ts-node/register --reporter spec tests/**/*.spec.ts", | ||
| "test:browser": "karma start", | ||
| "test": "npm run build && npm run lint && npm run test:node && npm run test:browser", |
There was a problem hiding this comment.
I prefer to put linting last / as a "posttest" because when developing it's nice to be able to run the tests without worrying about linting for fast iteration
|
@balena-ci retest |
| @@ -9,6 +9,9 @@ | |||
| "sourceMap": true, | |||
| "strictNullChecks": true, | |||
| "target": "es5", | |||
There was a problem hiding this comment.
I'd appreciate seeing @balena/es-version used but for now this is ok
Change-type: minor
Change-type: patch
ghost
merged commit 
Along with balena-io-modules/fetch-google-apps-script-ponyfill#1 this enables us to use pineclient in Google Apps Script environments (for example use pine inside google sheets to populate fields) 🎉
Change-type: minor
See: https://www.flowdock.com/app/rulemotion/p-billing/threads/cNz2yByrqr3g-5QDsusLJAY-RwZ