search

Agoric / dapp-card-store

A example dapp for selling baseball cards and other NFTs
29 stars 16 forks source link

Proposal: Use Next.js demo #37

Open ctjlewis opened 3 years ago

ctjlewis commented 3 years ago

I've forked this as I'm looking into submitting a proposal for the Gitcoin contract—would this team be horribly opposed to me replacing the Parcel + SPA with a Next.js demo?

Would make routing a little more simple, plus a lot of Web3 products (most finance ones I know of, especially those built on Ethereum) are Next projects.

ctjlewis commented 3 years ago

Little tougher than it sounds. After copy-pasting App.jsx into Next's _app.tsx equivalent, it throws because @agoric/captp in App.jsx expects SES to be available (I think):

assert is not defined 
$ yarn dev -p 3001
yarn run v1.22.5
$ next dev -p 3001
ready - started server on 0.0.0.0:3001, url: http://localhost:3001
info  - Using webpack 5. Reason: Enabled by default https://nextjs.org/docs/messages/webpack5
event - compiled successfully
event - build page: /
wait  - compiling...
event - compiled successfully
event - build page: /next/dist/pages/_error
wait  - compiling...
event - compiled successfully
ReferenceError: assert is not defined
    at eval (webpack-internal:///../../../agoric-sdk/packages/marshal/src/helpers/passStyleHelpers.js:24:34)
    at Object.../../../agoric-sdk/packages/marshal/src/helpers/passStyleHelpers.js (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:2136:1)
    at __webpack_require__ (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///../../../agoric-sdk/packages/marshal/index.js:23:90)
    at Object.../../../agoric-sdk/packages/marshal/index.js (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:2070:1)
    at __webpack_require__ (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///../../../agoric-sdk/packages/captp/src/index.js:7:73)
    at Object.../../../agoric-sdk/packages/captp/src/index.js (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:1993:1)
    at __webpack_require__ (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///./src/pages/_app.tsx:7:71)
ReferenceError: assert is not defined
    at eval (webpack-internal:///../../../agoric-sdk/packages/marshal/src/helpers/passStyleHelpers.js:24:34)
    at Object.../../../agoric-sdk/packages/marshal/src/helpers/passStyleHelpers.js (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:2136:1)
    at __webpack_require__ (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///../../../agoric-sdk/packages/marshal/index.js:23:90)
    at Object.../../../agoric-sdk/packages/marshal/index.js (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:2070:1)
    at __webpack_require__ (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///../../../agoric-sdk/packages/captp/src/index.js:7:73)
    at Object.../../../agoric-sdk/packages/captp/src/index.js (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:1993:1)
    at __webpack_require__ (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///./src/pages/_app.tsx:7:71)
error - ../../../agoric-sdk/packages/marshal/src/helpers/passStyleHelpers.js (10:31) @ eval
ReferenceError: assert is not defined
   8 | import '@agoric/assert/exported.js';
   9 | 
> 10 | const { details: X, quote: q } = assert;
     |                               ^
  11 | const {
  12 |   getOwnPropertyDescriptor,
  13 |   hasOwnProperty: objectHasOwnProperty,

So I add import '@agoric/install-ses' to top of _app.tsx, and now it complains about the harden identifier:

harden is not defined 
$ yarn dev -p 3001
yarn run v1.22.5
$ next dev -p 3001
ready - started server on 0.0.0.0:3001, url: http://localhost:3001
info  - Using webpack 5. Reason: Enabled by default https://nextjs.org/docs/messages/webpack5
event - compiled successfully
event - build page: /
wait  - compiling...
event - compiled successfully
Removing intrinsics.Object.getOwnPropertyNames.prototype
failed to delete intrinsics.Object.getOwnPropertyNames.prototype (TypeError#1)
TypeError#1: Cannot delete property 'prototype' of function getOwnPropertyNames(e){for(var t=f(e),r=0,n=0,s=t.length;rn){t[n]=t[r......t}

  at visitProperties (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9815:22)
  at isAllowedPropertyValue (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9691:7)
  at isAllowedProperty (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9751:14)
  at visitProperties (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9798:26)
  at isAllowedPropertyValue (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9691:7)
  at isAllowedProperty (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9751:14)
  at visitProperties (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9798:26)
  at whitelistIntrinsics (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9832:3)
  at repairIntrinsics (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:10124:3)
  at lockdown (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:10187:35)

event - build page: /next/dist/pages/_error
wait  - compiling...
event - compiled successfully
Removing intrinsics.Object.getOwnPropertyNames.prototype
failed to delete intrinsics.Object.getOwnPropertyNames.prototype (TypeError#1)
TypeError#1: Cannot delete property 'prototype' of function getOwnPropertyNames(e){for(var t=f(e),r=0,n=0,s=t.length;rn){t[n]=t[r......t}

  at visitProperties (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9815:22)
  at isAllowedPropertyValue (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9691:7)
  at isAllowedProperty (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9751:14)
  at visitProperties (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9798:26)
  at isAllowedPropertyValue (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9691:7)
  at isAllowedProperty (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9751:14)
  at visitProperties (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9798:26)
  at whitelistIntrinsics (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:9832:3)
  at repairIntrinsics (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:10124:3)
  at lockdown (webpack-internal:///../../../agoric-sdk/node_modules/ses/dist/ses.umd.js:10187:35)

(TypeError#1)
error - (TypeError#2)
TypeError#2: Cannot delete property 'prototype' of function getOwnPropertyNames(e){for(var t=f(e),r=0,n=0,s=t.length;rn){t[n]=t[r......t}

(ReferenceError#3)
ReferenceError#3: harden is not defined

  at eval (webpack-internal:///../../../agoric-sdk/packages/install-ses/install-ses.js:151:1)
  at Object.../../../agoric-sdk/packages/install-ses/install-ses.js (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:2103:1)
  at __webpack_require__ (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/webpack-runtime.js:33:42)
  at eval (webpack-internal:///./src/pages/_app.tsx:5:77)
  at Object../src/pages/_app.tsx (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:1313:1)
  at __webpack_require__ (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/webpack-runtime.js:33:42)
  at __webpack_exec__ (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:2333:39)
  at /home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:2334:28
  at Object. (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:2337:3)

Just doing import 'ses' throws:

Cannot harden before lockdown 
$ yarn dev -p 3001
yarn run v1.22.5
$ next dev -p 3001
ready - started server on 0.0.0.0:3001, url: http://localhost:3001
info  - Using webpack 5. Reason: Enabled by default https://nextjs.org/docs/messages/webpack5
event - compiled successfully
event - build page: /
wait  - compiling...
event - compiled successfully
event - build page: /next/dist/pages/_error
wait  - compiling...
event - compiled successfully
Error: Cannot harden before lockdown
    at makeError (/home/christian/PersonalProjects/dapp-card-store-bak/node_modules/ses/dist/ses.cjs:3076:17)
    at fail (/home/christian/PersonalProjects/dapp-card-store-bak/node_modules/ses/dist/ses.cjs:3195:20)
    at baseAssert (/home/christian/PersonalProjects/dapp-card-store-bak/node_modules/ses/dist/ses.cjs:3213:13)
    at harden (/home/christian/PersonalProjects/dapp-card-store-bak/node_modules/ses/dist/ses.cjs:5115:3)
    at eval (webpack-internal:///../../../agoric-sdk/packages/marshal/src/helpers/passStyleHelpers.js:34:1)
    at Object.../../../agoric-sdk/packages/marshal/src/helpers/passStyleHelpers.js (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:2147:1)
    at __webpack_require__ (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///../../../agoric-sdk/packages/marshal/index.js:23:90)
    at Object.../../../agoric-sdk/packages/marshal/index.js (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:2081:1)
    at __webpack_require__ (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/webpack-runtime.js:33:42)
Error: Cannot harden before lockdown
    at makeError (/home/christian/PersonalProjects/dapp-card-store-bak/node_modules/ses/dist/ses.cjs:3076:17)
    at fail (/home/christian/PersonalProjects/dapp-card-store-bak/node_modules/ses/dist/ses.cjs:3195:20)
    at baseAssert (/home/christian/PersonalProjects/dapp-card-store-bak/node_modules/ses/dist/ses.cjs:3213:13)
    at harden (/home/christian/PersonalProjects/dapp-card-store-bak/node_modules/ses/dist/ses.cjs:5115:3)
    at eval (webpack-internal:///../../../agoric-sdk/packages/marshal/src/helpers/passStyleHelpers.js:34:1)
    at Object.../../../agoric-sdk/packages/marshal/src/helpers/passStyleHelpers.js (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:2147:1)
    at __webpack_require__ (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///../../../agoric-sdk/packages/marshal/index.js:23:90)
    at Object.../../../agoric-sdk/packages/marshal/index.js (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:2081:1)
    at __webpack_require__ (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/webpack-runtime.js:33:42)
error - Error: Cannot harden before lockdown
    at makeError (/home/christian/PersonalProjects/dapp-card-store-bak/node_modules/ses/dist/ses.cjs:3076:17)
    at fail (/home/christian/PersonalProjects/dapp-card-store-bak/node_modules/ses/dist/ses.cjs:3195:20)
    at baseAssert (/home/christian/PersonalProjects/dapp-card-store-bak/node_modules/ses/dist/ses.cjs:3213:13)
    at harden (/home/christian/PersonalProjects/dapp-card-store-bak/node_modules/ses/dist/ses.cjs:5115:3)
    at eval (webpack-internal:///../../../agoric-sdk/packages/marshal/src/helpers/passStyleHelpers.js:34:1)
    at Object.../../../agoric-sdk/packages/marshal/src/helpers/passStyleHelpers.js (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:2147:1)
    at __webpack_require__ (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///../../../agoric-sdk/packages/marshal/index.js:23:90)
    at Object.../../../agoric-sdk/packages/marshal/index.js (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/pages/_app.js:2081:1)
    at __webpack_require__ (/home/christian/PersonalProjects/dapp-card-store-bak/demo/.next/server/webpack-runtime.js:33:42) {
  page: '/'
}
ctjlewis commented 3 years ago

So the reason it jumps from assert to harden not being defined is because the import 'ses' statement fails in whatever weird Webpack context Next spins up, so it throws while trying to execute and harden is not successfully defined.

If this team would not be adamantly opposed to considering a Next demo, I can help figure out what is needed to properly set up an SES context in Next so that we can render an Agoric app as expected. Then you can have a cool little Deploy button in the README.

samsiegart commented 3 years ago

Hi C, we are not opposed to Next.js, thanks for bringing this up. We would actually be interested in getting this to work given its ubiquity as you mentioned, so we'd greatly appreciate any effort you could put into this.

I have not tried Next.js, but I was able to get a plain React app working (https://github.com/Agoric/agoric-sdk/pull/3890). Generally, we need to import and run lockdown before any other scripts run. We pull the library in at the top of index.html and call it there (though I'm not sure if this works in Next.js).

We noticed that if we try to import lockdown in index.js, in prod builds the library is minified which ends up breaking it, hence why we include it directly in index.html.

We also noticed that without certain arguments (__allowUnsafeMonkeyPatching__: 'unsafe', and for dev, consoleTaming: 'unsafe'), React breaks because it tries to modify certain locked-down objects.

Hopefully this helps a bit.

ctjlewis commented 3 years ago

@samsiegart Thank you Sam. This integration is actually a little tricky—the problem is that the core App component needs things like @agoric/captp, which expect SES globals like assert and so on, so I will need to figure out how to get an SES context working in a Next SSR context. For instance:

import '@agoric/install-ses';
export default function App() { return null; };

Produces the following error:

https://gist.github.com/ctjlewis/11aab92a235ba694b67e857b08f4a731

It seems like the SES shim is working against other runtime settings Next is relying on. This leaves us with three options as I understand it:

  1. Find a way to get the SES shim to work in Next, i.e. resolve Cannot delete property 'prototype' of function getOwnPropertyNames (hard)
  2. Update the @agoric/captp and other modules to include import 'ses' statements or whatever is needed to make sure that the modules are truly modular and contain all the members they will need at runtime (preferred). (This issue is really driven by the modules needing us to have an SES context prior to importing.)
  3. Simply abandon adding support :-(
erights commented 3 years ago

Hi @ctjlewis that error log is most helpful. Thanks!

failed to delete intrinsics.Object.getOwnPropertyNames.prototype

This error message indicate that, prior to SES lockdown something else (Next?) ran and monkey patched Object.getOwnPropertyNames with a function they wrote using the function keyword. Builtin methods, arrow functions, and concise methods have no prototype property. They almost certainly do not make use of this accidental prototype property, and so could switch to another function like an arrow function or concise method.

To diagnose what in Next is doing this, if you initialize Next after SES lockdown, without setting {__allowUnsafeMonkeyPatching__: 'unsafe'}, then you'll instead get an error when Next tries to replaceObject.getOwnPropertyNames`. The stack trace should tell us what in Next we need to fix. We can fix it first with a local patch (as Agoric does for other things at https://github.com/Agoric/agoric-sdk/tree/master/patches ) until we get Next to accept the fix upstream.

Best would be if they did not monkey patch, a known terrible software engineering malpractice, and were able to successfully initialize after SES lockdown. Once we know what specifically in Next needs to be fixed, we should add it to our tracking bug at https://github.com/endojs/endo/issues/576 . Thanks.

ctjlewis commented 3 years ago

Super sorry @erights, I have seen the __allowUnsafeMonkeyPatching__ flag passed somewhere in the codebase, but do not remember exactly how it is used. Can you help me refactor the example with @agoric/install-ses above to achieve that?

Do you mean something like this:

import 'ses'
lockdown({}) // ?
erights commented 3 years ago

Do you mean something like this:

Yes, exactly.

ctjlewis commented 3 years ago

With:

import 'ses';
lockdown({});
export default function App() { return <h1>Test</h1>; };

On the server side, we still see the same error trying to render the component:

yarn run v1.22.5
$ next dev
ready - started server on 0.0.0.0:3000, url: http://localhost:3000
info  - Using webpack 5. Reason: Enabled by default https://nextjs.org/docs/messages/webpack5
event - compiled successfully
event - build page: /next/dist/pages/_error
wait  - compiling...
event - compiled successfully
Removing intrinsics.Object.getOwnPropertyNames.prototype
failed to delete intrinsics.Object.getOwnPropertyNames.prototype (TypeError#1)
TypeError#1: Cannot delete property 'prototype' of function getOwnPropertyNames(e){for(var t=p(e),r=0,n=0,s=t.length;r<s;++r){if(!i.call(c,t[r])){if(r>n){t[n]=t[r...<omitted>...t}

(TypeError#1)
event - build page: /
wait  - compiling...
event - compiled successfully

But the page loads, and it's locked, so in the console I can do harden({}) with no issues related to calling harden before lockdown.

But the render is not successful. The logs are very ugly, but I think this one is the problem, indicating the lockdown breaks something with next-dev.js from the Next library:

The above error occurred in the <Head> component:

Error generating stack: Cannot redefine property: info

Full log: https://gist.github.com/ctjlewis/3a0082c020e859dfee8f04b71b0db2aa

erights commented 3 years ago

Hi @ctjlewis the following from your log makes me think this needs dedicated time by someone with adequate knowledge of the insides of SES and of Next:

console.js?eb9e:321 Removing intrinsics.Symbol.observable
console.js?eb9e:321 Removing intrinsics.Reflect.decorate
console.js?eb9e:321 Removing intrinsics.Reflect.metadata
console.js?eb9e:321 Removing intrinsics.Reflect.defineMetadata
console.js?eb9e:321 Removing intrinsics.Reflect.hasMetadata
console.js?eb9e:321 Removing intrinsics.Reflect.hasOwnMetadata
console.js?eb9e:321 Removing intrinsics.Reflect.getMetadata
console.js?eb9e:321 Removing intrinsics.Reflect.getOwnMetadata
console.js?eb9e:321 Removing intrinsics.Reflect.getMetadataKeys
console.js?eb9e:321 Removing intrinsics.Reflect.getOwnMetadataKeys
console.js?eb9e:321 Removing intrinsics.Reflect.deleteMetadata

This is some major monkey patching. This trace results from Next initializing before lockdown. Next added all these non-standard properties to one of the fundamental intrinsic objects, the Reflect object. SES did its job and removed all of them during lockdown. However, the chances that Next would still work after its extensive monkey patches were removed is slim. If you did it in the other order (which I still recommend trying) of doing lockdown before Next initializes, then it will fail to add these properties. But unlike Object.getOwnProperties.prototype these are unlikely to be accidental additions, where we could easily repair Next to do without them. Rather, either Next's failure to add them (if after lockdown) or SES's removal of them (if before lockdown) are likely to break Next in ways that are not easily repaired. Based on this portion of the log alone, I'd have to label Next as something unlikely to become SES compatible anytime soon. It is quite a shame. Co-existence would have been great!

Please someone who knows the Next people, please let them know of this roadblock. They might have some advice.

erights commented 3 years ago

@ctjlewis I appreciate the time and energy you put into this. It was a good idea and worth a good try. I am sorry it did not work out.

ctjlewis commented 3 years ago

Thank you very much for those notes Mark.

It's definitely a super complicated problem but your notes added context, I'm not giving up on it yet. I will think about this more over the next few days. Also I know the Next people quite well, I will ping the contributor Slack and see if I can get some feedback.

Seems like we're on the right path and what's needed is sitting down and figuring out how we can use an SES context for a server-side rendered component without tampering with Next's own context—it seems like they're all just one thing, where next-dev.js probably just loads the module, renders the component, and spits it out without isolating it in any way?

In theory, the actual program's context should differ from Next's so that this doesn't happen (we are talking about Next's design here). Importing whatever at the top of our app and modifying runtime globals should all happen inside our app, and not the Next server (or build process) itself.

What I mean is, right now, we can write a program right now that we can execute with node pages/_app.js and get no errors, but if we ask Next to compile it, it breaks because it shares the same context as the program and Next's internal logic is unsurprisingly not SES compatible.

Pretty complicated but I think I'll make a breakthrough on this over the next few days hopefully. I wish I was more familiar with SES because it would help, but your notes have been super helpful. I will try to formalize what the source of this issue is and write some notes on how we can server-side render React components in an SES context with an emphasis on Next, will also bring up in Endo repo as you mentioned.

erights commented 3 years ago

Glad to hear it! Please ask questions as they arise. As long you as you keep trying, I'll keep answering your questions ;)

One thing I am confused about, that may make my previous diagnosis wrong in any case, is what runs where. If Next was running only on the client (which makes sense) and the trace came from the server, then those weird properties on Reflect were added by something on the server, which therefore isn't Next. What server are we talking about? What's running on it?

ctjlewis commented 3 years ago

So, Next will render the component in a Node context on the server (that's the "server-side rendering," or SSR), and then it ships the "skeleton" render to the client where it hydrates with props.

So there were two logs there:

I will ask Next team if they have any thoughts here. My first thought is that the component and the Next server rendering the component should surely not share the same context like they seem to be, right? This is about Next's design really more than SES but, just for a second opinion, isn't that a vulnerability at worst, room for issues like this at best (where the app will run in its own context via node pages/_app.js with no issues, but Next cannot render it because Next is predictably not SES-compatible)?

kriskowal commented 3 years ago

It will help our case considerably when we find time to support vetted shims for SES. SES has an internal description of what properties of the shared intrinsics to permit after lockdown and vetted shims need to be able to rearrange that to their design. It is very likely that a future version of SES will need to expose the permits graph. But, equal caution must go into the modification of the permits graph as has gone into the design of SES itself in order to preserve the security invariants we currently provide.

dckc commented 3 years ago

It will help our case considerably when we find time to support vetted shims for SES.

In case @ctjlewis wants to watch the progress, that's https://github.com/endojs/endo/issues/422 , right @kriskowal ?

kriskowal commented 3 years ago

Yes.

On Thu, Sep 30, 2021 at 10:58 AM Dan Connolly @.***> wrote:

It will help our case considerably when we find time to support vetted shims for SES.

In case @ctjlewis https://github.com/ctjlewis wants to watch the progress, that's endojs/endo#422 https://github.com/endojs/endo/issues/422 , right @kriskowal https://github.com/kriskowal ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Agoric/dapp-card-store/issues/37#issuecomment-931541715, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAOXBUHLWAT2MS5J7S6NXDUESQMDANCNFSM5EZG7XUA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

ctjlewis commented 3 years ago

FYI: I have found a solution for this and will update later. It simply involves sandboxing page imports in Next with vm.Script. Currently, your page (pages/_app.js) and Next (via next/server/load-components and next/server/require) share the same context, so SES breaks Next server itself and the process fails.

I just tested this and I was able to get a reference to the imported page with SES shims using this sandboxing approach. Will provide a fuller update later today after I send a PR in to Next.

ctjlewis commented 3 years ago

Update: Half-working

The module isolation approach did resolve half of the problem: This Next fork can now compile SES-locked modules without breaking (in a production context, see below), as they are now properly restricted to their own context.

Links:

Production mode

Works as expected, both with import 'ses'; lockdown({}) and import '@agoric/install-ses! Component renders and while I'm getting some errors from browser extensions because of the lockdown, the app logic is executed properly.

In the demo, you can see that the console logs the output of a test harden({ ... }) call at runtime and the page renders correctly.

Dev mode

Here is where I have not had much time to think about possible approaches and would definitely appreciate thoughts, but basically: Naturally, the dev server logic (which involves hot refresh and a dev overlay for errors) is substantially less straightforward than the build process (which just imports the components and renders them), and the SES runtime conflicts with the business logic of Next's dev server, specifically with the Webpack runtime.

After working on it for a few days, I believe the problem is that the SES shim breaks a specific utility used by the Webpack runtime called __webpack_require__(...), which is used by Next to resolve hot reloads in the browser and powers the dev server.

You can see that it is just a file called webpack-runtime.js that is emitted by Next into the .next/server build dir. A sample is available as a gist for ease of review: https://gist.github.com/ctjlewis/5477c3f433cd6307a6ba0b2e7d607a36

An example is included as test.mjs in the repro:

/**
 * Import the Webpack runtime.
 */
const { default: __webpack_require__ } = await import('./.next/server/webpack-runtime.js')
console.log('Before SES:', { __webpack_require__ })

/**
 * Load SES and observe how it has modified __webpack_require__.
 */
await import('@agoric/install-ses')
console.log('After SES:', { __webpack_require__ })

Output:

Before SES: {
  __webpack_require__: [Function: __webpack_require__] {
    m: {},
    a: [Function (anonymous)],
    n: [Function (anonymous)],
    d: [Function (anonymous)],
    f: { require: [Function (anonymous)] },
    e: [Function (anonymous)],
    u: [Function (anonymous)],
    o: [Function (anonymous)],
    r: [Function (anonymous)],
    X: [Function (anonymous)],
    C: [Function: installChunk]
  }
}
Removing intrinsics.Object.hasOwn
Removing intrinsics.%EvalErrorPrototype%.cause
Removing intrinsics.%RangeErrorPrototype%.cause
Removing intrinsics.%ReferenceErrorPrototype%.cause
Removing intrinsics.%SyntaxErrorPrototype%.cause
Removing intrinsics.%TypeErrorPrototype%.cause
Removing intrinsics.%URIErrorPrototype%.cause
Removing intrinsics.%ErrorPrototype%.cause
After SES: {
  __webpack_require__: {
    m: {},
    a: {},
    n: {},
    d: {},
    f: { require: {} },
    e: {},
    u: {},
    o: {},
    r: {},
    X: {},
    C: {}
  }
}

So, at the moment, SES shims do not break or preclude the SSR process as long as the requires are isolated as in the draft Next PR above, but they do break the Webpack-powered dev server.