-
Notifications
You must be signed in to change notification settings - Fork 103
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
Imports #197
Conversation
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.
Minor readability edits
Co-authored-by: Crystal Gomes <[email protected]>
Co-authored-by: Crystal Gomes <[email protected]>
README.md
Outdated
|
||
> ⚠️ **Users are fully responsible for any dependencies their JavaScript source code imports. Chainlink is not responsible for any imported dependencies and provides no guarantees of the validity, availability or security of any libraries a user chooses to import. Developers are advised to fully vet any imported dependencies or avoid dependencies altogether to avoid any risks associated with a compromised library or a compromised repository from which the dependency is downloaded.** | ||
|
||
Chainlink Functions supports importing ESM-compatible modules with are supported by Deno within the JavaScript source code. It also supports importing some NPM packages [via the `npm:` specifier](https://docs.deno.com/runtime/manual/node/npm_specifiers) and some built-in Node.js modules [via the `node:` specifier](https://docs.deno.com/runtime/manual/node/node_specifiers). Check out the [Deno documentation on importing modules](https://docs.deno.com/runtime/manual/basics/modules/) for more information or visit [deno.land/x](https://deno.land/x) to find 3rd party modules which have been built for Deno. |
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.
nit: is Nodejs standard library a better way to indicate "built in"? I'm divided on this so either way works.
README.md
Outdated
|
||
> ⚠️ **Users are fully responsible for any dependencies their JavaScript source code imports. Chainlink is not responsible for any imported dependencies and provides no guarantees of the validity, availability or security of any libraries a user chooses to import. Developers are advised to fully vet any imported dependencies or avoid dependencies altogether to avoid any risks associated with a compromised library or a compromised repository from which the dependency is downloaded.** | ||
|
||
Chainlink Functions supports importing ESM-compatible modules with are supported by Deno within the JavaScript source code. It also supports importing some NPM packages [via the `npm:` specifier](https://docs.deno.com/runtime/manual/node/npm_specifiers) and some built-in Node.js modules [via the `node:` specifier](https://docs.deno.com/runtime/manual/node/node_specifiers). Check out the [Deno documentation on importing modules](https://docs.deno.com/runtime/manual/basics/modules/) for more information or visit [deno.land/x](https://deno.land/x) to find 3rd party modules which have been built for Deno. |
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.
This para could be broken up? Example:
You can import
- ESM-compatible NPM packages via the
npm:
specifier - ESM-compatible built-in Node.js modules via the
node:
specifier - ESM-Compatible 3rd party modules which have been built for Deno from deno.land/x
Check out the Deno documentation on importing modules for more information
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.
Yes, I agree with this, except make it an unordered (bullet point) list
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.
@thedriftofwords @zeuslawyer This is good feedback. @thedriftofwords Would you be able to break up this paragraph on this PR? Else maybe I can do it later tonight.
|
||
All other execution restrictions still apply to imported dependencies. This means the dependencies will not have access to the file system, environment variables or any other Deno permissions. If an imported library requires restricted permissions, importing the library may result in an error. Furthermore, dependencies are downloaded at runtime, meaning the time required to download a dependency is counted toward the total JavaScript source code execution time limit. | ||
|
||
Sometimes imported dependencies use additional fetch requests to load additional code or resources. These fetch requests count toward the total number of HTTP requests that the JavaScript source code is allowed to perform. If the imported dependencies exceed this total number of allowed fetch requests, the import attempt will fail with an error. |
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.
Recommend we link to the docs (https://docs.chain.link/chainlink-functions/resources/service-limits) in the test "allowed to perform".
And/or expressly use the phrase "service limits" so that it maps in the readers head to what they see in the docs.
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.
LGTM
No description provided.