Skip to content
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

Ses initial draft review #437

Merged
merged 56 commits into from Mar 18, 2021
Merged

Ses initial draft review #437

merged 56 commits into from Mar 18, 2021

Conversation

tyg
Copy link
Contributor

@tyg tyg commented Feb 15, 2021

No description provided.

@tyg tyg self-assigned this Feb 15, 2021
@tyg tyg requested a review from dckc February 15, 2021 18:45
@dckc
Copy link
Member

dckc commented Feb 15, 2021

I'm new to reviewing docs. The "files changed" view here has lots of distracting markup and doesn't make clear what context is available to the reader.

CONTRIBUTING.md says

  1. Check your changed docs in a local copy of the repo and local documentation server.

but I don't see details on how to start a local documentation server.

The yarn.lock suggests yarn install should get dependencies, and it runs without error, but then the conventional yarn start loses with error Command "start" not found. Inspecting package.json suggests yarn docs:dev but that fails with vuepress: not found.

Suggestions?

Copy link
Member

@kriskowal kriskowal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This review covers all of the **tyg blocks I could find. Please as always let me know if I missed a question!

main/javascript-modifications/lockdown.md Outdated Show resolved Hide resolved
Comment on lines 77 to 78
**tyg todo Not sure what the current situation is with respect to what needs to go into
what code to get and use SES in one's code**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(With regret, I can’t propose a suggestion since the suggestion contains code fences. So here’s my suggestion inline.)

On Node.js and in web browsers, use the SES shim to transform an ordinary JavaScript environment into a SES environment.

On Node.js you can import or require ses in either CommonJS or ECMAScript modules, then call lockdown. This is a “shim”, meaning that it mutates the environment in place such that any code running after the shim can assume that it’s running in a SES environment, one that includes the globals lockdown, harden, Compartment, and so on.

require("ses");
lockdown();
import 'ses';
lockdown();

To ensure that a module gets run in a SES environment, wrap the text above in a ses-lockdown.js module and import that.

import './non-ses-code-before-lockdown.js';
import './ses-lockdown.js'; // calls lockdown.
import './ses-code-after-lockdown.js';

To use SES as a script on the web, use the UMD build.

<script src="node_modules/ses/dist/ses.umd.min.js">

The full SES environment includes a module system that requires a translating compiler. To use the much thinner version of SES that just provides an evaluator Compartment, use the ses/lockdown layer. It is much smaller and useful especially if all client code gets bundled as a script out-of-band.

import 'ses/lockdown';
<script src="node_modules/ses/dist/lockdown.umd.min.js"></script>

main/javascript-modifications/ses-guide.md Outdated Show resolved Hide resolved
main/javascript-modifications/lockdown.md Outdated Show resolved Hide resolved
main/javascript-modifications/lockdown.md Outdated Show resolved Hide resolved
main/javascript-modifications/ses-guide.md Outdated Show resolved Hide resolved
main/javascript-modifications/ses-guide.md Outdated Show resolved Hide resolved
main/javascript-modifications/ses-reference.md Outdated Show resolved Hide resolved
main/javascript-modifications/ses-reference.md Outdated Show resolved Hide resolved
main/javascript-modifications/ses-reference.md Outdated Show resolved Hide resolved
@Agoric Agoric deleted a comment from kriskowal Mar 18, 2021
@tyg tyg closed this Mar 18, 2021
@tyg tyg reopened this Mar 18, 2021
@tyg tyg marked this pull request as ready for review March 18, 2021 21:49
@tyg tyg merged commit 82f7a98 into main Mar 18, 2021
@tyg tyg deleted the SES branch March 18, 2021 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants