-
Notifications
You must be signed in to change notification settings - Fork 16
Proposal: Use Next.js demo #37
Comments
Little tougher than it sounds. After copy-pasting
|
So the reason it jumps from 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. |
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 (Agoric/agoric-sdk#3890). Generally, we need to import and run We noticed that if we try to import lockdown in We also noticed that without certain arguments ( Hopefully this helps a bit. |
@samsiegart Thank you Sam. This integration is actually a little tricky—the problem is that the core App component needs things like 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:
|
Hi @ctjlewis that error log is most helpful. Thanks!
This error message indicate that, prior to SES lockdown something else (Next?) ran and monkey patched To diagnose what in Next is doing this, if you initialize Next after SES lockdown, without setting Best would be if they did not monkey patch, a known terrible software engineering malpractice, and were able to successfully initialize after SES |
Super sorry @erights, I have seen the Do you mean something like this: import 'ses'
lockdown({}) // ? |
Yes, exactly. |
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:
But the page loads, and it's locked, so in the console I can do 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
Full log: |
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:
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 Please someone who knows the Next people, please let them know of this roadblock. They might have some advice. |
@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. |
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 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 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. |
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 |
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 |
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. |
In case @ctjlewis wants to watch the progress, that's endojs/endo#422 , right @kriskowal ? |
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
<endojs/endo#422> , right @kriskowal
<https://github.com/kriskowal> ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#37 (comment)>,
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>.
|
FYI: I have found a solution for this and will update later. It simply involves sandboxing page imports in Next with 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. |
Update: Half-workingThe 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 modeWorks as expected, both with In the demo, you can see that the console logs the output of a test Dev modeHere 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 You can see that it is just a file called An example is included as /**
* 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:
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. |
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.
The text was updated successfully, but these errors were encountered: