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

feat(wallet): Create react ui scaffold with ses #3879

Merged
merged 3 commits into from
Sep 23, 2021
Merged

Conversation

samsiegart
Copy link
Contributor

No description provided.

Copy link
Member

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

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

LGTM! Definitely a bare-bones starting point.

packages/dapp-svelte-wallet/react-ui/package.json Outdated Show resolved Hide resolved
<meta charset="utf-8" />
<script src="lockdown.umd.js"></script>
<script>
if ('%NODE_ENV%' === 'production') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we allow this to run in dev mode, we get the following error:

Uncaught TypeError: Cannot add property reactStack, object is not extensible
  at p (index.js:1)
  at K (index.js:1)
  at Module.de (index.js:1)
  at Object../node_modules/react-dev-utils/webpackHotDevClient.js (webpackHotDevClient.js:45)
  at __webpack_require__ (bootstrap:851)
  at fn (bootstrap:150)
  at Object.1 (logo.svg:1)
  at __webpack_require__ (bootstrap:851)
  at checkDeferredModules (bootstrap:45)
  at Array.webpackJsonpCallback [as push] (bootstrap:32)
  at main.chunk.js:1

image

Copy link
Member

Choose a reason for hiding this comment

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

Hi @samsiegart could you either (or both) include the error text (not screenshot) when run against an uncompressed react? Or explain here how to reproduce.

@kriskowal @mhofman @dckc doesn't look like an override mistake. Looks like it is genuinely trying to monkey patch something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The node module itself comes already compressed, I might have some trouble trying to get an uncompressed version. To repro, you can remove the surrounding if ('%NODE_ENV%' === 'production') so that it runs in dev mode.

Then cd agoric-sdk/packages/dapp-svelte-wallet, agoric install, cd react-ui, yarn install and yarn start to run local dev mode.

Copy link
Member

@mhofman mhofman Sep 23, 2021

Choose a reason for hiding this comment

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

to be clear, this error happens in production mode / once built outside of a dev server? I'm only half surprised react attempts to monkey patch error (I remember reading something about them trying to capture the error stack)

Copy link
Member

Choose a reason for hiding this comment

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

this error happens in production mode / once built outside of a dev server?

The opposite. The error only happens in non-prod mode if the code that's currently guarded by the if statement runs without that guard.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This appears to be the culprit from the package source code (wasn't able to figure out how to import and run it uncompressed though): https://github.com/facebook/create-react-app/blob/bb64e31a81eb12d688c14713dce812143688750a/packages/react-error-overlay/src/effects/proxyConsole.js

@samsiegart samsiegart merged commit 089c876 into master Sep 23, 2021
@samsiegart samsiegart deleted the wallet-react-ui branch September 23, 2021 20:45
@erights
Copy link
Member

erights commented Sep 24, 2021 via email

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