Conversation
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: xtend@4.0.2 |
|
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again. Next stepsWhat is filesystem access?Accesses the file system, and could potentially read sensitive data. If a package must read the file system, clarify what it will read and ensure it reads only what it claims to. If appropriate, packages can leave file system access to consumers and operate on data passed to it instead. Why is contributor and author data important?Package does not specify a list of contributors or an author in package.json. Add a author field or contributors array to package.json. What are unmaintained packages?Package has not been updated in more than a year and may be unmaintained. Problems with the package may go unaddressed. Package should publish periodic maintenance releases if they are maintained, or deprecate if they have no intention in further maintenance. What is network access?This module accesses the network. Packages should remove all network access that is functionally unnecessary. Consumers should audit network access to ensure legitimate use. What is shell access?This module accesses the system shell. Accessing the system shell increases the risk of executing arbitrary code. Packages should avoid accessing the shell which can reduce portability, and make it easier for malicious shell access to be introduced. What is new author?A new npm collaborator published a version of the package for the first time. New collaborators are usually benign additions to a project, but do indicate a change to the security surface area of a package. Scrutinize new collaborator additions to packages because they now have the ability to publish code into your dependency tree. Packages should avoid frequent or unnecessary additions or changes to publishing rights. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
mcmire
force-pushed
the
convert-to-typescript
branch
from
f756c34 to
4b32637
Compare
- Convert the existing JavaScript code to TypeScript, removing the need for a type definitions file - Add typedoc to generate JSDocs - BREAKING: Change default export to a named export to align with other libraries - Export `Provider` type, as `controller-utils` and other packages within `core` import it
mcmire
force-pushed
the
convert-to-typescript
branch
from
4b32637 to
76a922a
Compare
| @@ -0,0 +1,8 @@ | |||
| declare module 'json-rpc-random-id' { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,656 @@ | |||
| import createRandomIdFactory from 'json-rpc-random-id'; | |||
There was a problem hiding this comment.
Since we are backfilling the types for this library here, I tested building the project to make sure that this import line doesn't show up to prevent the same sort of error that we are having with eth-query, and while it does appear in the compiled JS, it does not appear in the .d.ts. So I think we're okay on this.
| if (response.result) { | ||
| return callback(null, response.result); | ||
| } | ||
| throw new Error( |
There was a problem hiding this comment.
This error does not get thrown in the original implementation, but I added it because of the type that we are assigning to the response:
type ProviderSendAsyncResponse<Result> = {
error?: { message: string };
result?: Result;
};In other words, both error and result can be undefined and so TypeScript indicates that not all code paths return a value.
Of course we control the type here. That said, I derived it from the implementation. It would be a little more accurate to use:
type ProviderSendAsyncResponse<Result> =
| {
error: { message: string }
}
| {
result: Result
};Or even:
type ProviderSendAsyncResponse<Result> =
| {
error: unknown
}
| {
result: Result
};but then we'd have to change the implementation.
| ) { | ||
| const callback = args.pop(); | ||
| // Typecast: The remaining arguments must be the params. | ||
| const params = args.slice(0, -1) as Params; |
There was a problem hiding this comment.
This is unfortunate, but I wasn't sure of another way to do this.
| const callback = args.pop(); | ||
| // Typecast: The remaining arguments must be the params. | ||
| const params = args.slice(0, -1) as Params; | ||
| if (callback === undefined) { |
There was a problem hiding this comment.
These checks are new. It makes sense that callback could be undefined because args may be empty. However, even though we've assigned a type of [...Params, SendAsyncCallback<Params>], TypeScript isn't willing to assume that callback is SendAsyncCallback<Params>, but gives it a type of SendAsyncCallback<Params> | Params[number].
|
All of the Socket warnings make sense: @SocketSecurity ignore vscode-oniguruma@1.7.0 |
|
I'm going to move this to draft. I don't like how I've defined the methods here (no reason to make them bound methods, and we might be able to assign better types to the individual methods that match the Ethereum spec). But also, I think the type definitions we have are good enough for now. |
|
While it looks great as such, I'm not sure if this is desirable for a base-library package like If this is revisited, perhaps it would be possible to port some of this to jsdoc and perhaps reconfigure tooling to get some of the benefits in this PR without introducing Typescript wholesale? |
2 participants
need for a type definitions file
libraries
Providertype, ascontroller-utilsand other packageswithin
coreimport itProgresses MetaMask/core#1471