-
Notifications
You must be signed in to change notification settings - Fork 74
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
App scaffolded with create-cosmos-app cannot call lockdown successfully #2033
Comments
This smells like a CommonJS interop issue and I would like to summon @naugtur, if we can borrow his eyes. |
@samsiegart Capturing the error in the description will help us diagnose. |
There's a screenshot and two stack traces in the example repo's README. I've updated the description here to point it out more explicitly |
Oh, it's private, I should make it public Edit: done |
@kriskowal , I've assigned it to the two of us. |
@naugtur , you too ;) |
Another data point- shouldn't be a surprise given I'm using the same |
Reproduced locally! Thanks for the clear and simple instructions. |
Yeah, @naugtur @kriskowal , it is all about the cjs loader. I will definitely need your help looking at this. It was easy to reproduce locally, so could you have a look? If the bug is interesting, perhaps we could discuss it at the upcoming endo meeting? |
Actually, this seems kinda urgent for us. Could we meet tomorrow after you've taken a first look? I am free tomorrow anytime after 2pm Pacific. |
@tgrecojs you mentioned on the call you might be familiar with this issue |
I just took a moment to spin up an example repo which integrates Pasting the contents of the README file from that repository below. @samsiegart let me know if this helps as all. One more thing I want to note - I think your repo may be using a version of past next.js. FWIW I have wired up past versions to work with endo so I don't believe that is the issue here but I'm not entirely sure. README.mdNext.js X @endo/init
This repository demonstrates the integration of Endo and Knit within a Next.js application. Below you can view contents of the project's _document.js file. import { Html, Head, Main, NextScript } from "next/document";
import Script from "next/script";
export default function Document() {
return (
<Html>
<Head />
<body>
<Main />
<NextScript />
<Script
async
type="module"
src="https://esm.sh/@endo/init@1.0.2"
strategy="beforeInteractive"
/>
</body>
</Html>
);
} The key component within this file is the Next.js specific <Script /> element as it is responsible for loading Checking for tamper-proof globals.Below is a screenshot of interaction from https://hardened-nextjs-repo.vercel.app |
I reproduced the error and the error starts on the server
the |
@naugtur did you get to that error using the _document.js file I referenced? If so, then I am definitely intersted in hearing as much. I reproduced the error as well in this repo and it did it by changing the I can see this my solution fixes this in the repo related to this issue if it would be most helpful. cc @samsiegart @erights |
@tgrecojs The difference between yours and this repo seems to be this one is running lockdown on the serverside and yours is running it on clientside.
|
This question is best suited for @samsiegart. It's my understanding that the intended result is for this error to go away, and for this application to properly run
I‘m assuming you are referring to the repo referenced in my last comment, and not the one I recreated last week. If you're referring to this project, then let me know how you were able to confirm lockdown was being called on the client. I pushed up a change last night after doing a bit of investigating now it seems as though the next.js docs on
This indicates that next.js to load this code before any hydration is done leading me to believe that hardened javascript would be loaded on the server before any other code (including next.js internals). There was one big issue though - The following code loads the script properly, but it still doesn't consistently download endo before next.js code.
Here's a repo to the latest implementation - A live URL can be viewed here - https://nextjs-with-endo-init-kpj7t18sm-thomasgreco.vercel.app/ |
It would also be preferred to be able to load |
I'm getting the impression that you may have viewed the repo I spun up as a potential solution to the issues your facing with create-cosmos-app. I want to quickly clarify that this was not my intention at all. It was only meant to provide some added context into integrating next.js with SES. So, as for your ask...
I can reference @endo/init locally without issue. But is that the only ask here, or am I missing something. If it's not, then please let me know what your end goal is so I can try and assist. |
Yea the intended result is that we can use endo APIs inside the application on top of next.js+create-cosmos-app. I guess if the local |
I came across this when trying to fork dapp-offer-up. I went around it by disabling server side rendering. |
The goal here seems to be using next.js' SSR capabilities and from what i've gathered the Script tag is the vehicle that will get us to the desired outcome. I've just drafted a PR for this here.. I'm adding the comments from that here for visibility.
endo-init-nextjs.mp4
@endo/init seems to be getting imported, but it just stops there resulting in I'm wondering if anyone can give additional input on how @endo/init is functioning here. I'm not sure why It seems like there is something I'm missing here but not exactly sure what 🤔 cc @naugtur @kriskowal @kumavis |
Hey @samsiegart . I've created a pull request] intended to fix some of these bugs, using a slightly modified Issues that still persist during development: Once you make a change on a file and attempt to reload page manually, WebPack will throw errors. After making changes you'll have to run Edit: I've hosted it here https://hardened-create-cosmos-app.vercel.app/ |
@gacogo this is a really interesting solution! I do have one question - are you still disabling SSR in your PR? Also, i'm not sure if you read any of my comments on this issue but I provide a fairly detailed look into how next.js suggests loading scripts (without disabling SSR). |
@tgrecojs Am not disabling ssr with this solution. I also tried dynamic imports with |
What's the status of this? Should this be tracked as an endo ecosystem compat issue #2037 ? |
@gacogo I tried to use you solution with:
But I still get the error:
does it work for you now? |
Describe the bug
As shown in https://github.com/agoric-labs/hardened-create-cosmos-app, when I try using
create-cosmos-app
to make a cosmos app (which runs on Next.js), I cannot successfully invokelockdown
and run the app.Error Details:
See the README of the example repo for error details.
Steps to reproduce
yarn && yarn dev
to install deps and run locallyyarn dev
Expected behavior
lockdown
can be invoked in the top-level of the application without error.Platform environment
Additional context
Create-cosmos-app is a tool for scaffolding cosmos apps with Next.js. It would be beneficial to be able to use SES and Endo APIs in apps built using this tool. The error is perhaps more general to Next.js as a whole, but this tool is of particular interest.
Screenshots
See https://github.com/agoric-labs/hardened-create-cosmos-app
The text was updated successfully, but these errors were encountered: