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

Update package-set to 0.15.4-20220808 and react dependency #294

Conversation

andys8
Copy link
Contributor

@andys8 andys8 commented Aug 10, 2022

Description of the change

Update to the latest package-set. Necessary to allow the latest react version v18 can be used cookbook examples


Checklist:

  • Added the change to the changelog's "Unreleased" section with a reference to this PR (e.g. "- Made a change (#0 by @)")
  • Linked any existing issues or proposals that this pull request should close
  • Updated or added relevant documentation
  • Added a test for the contribution (if applicable)

Necessary to allow the latest react version v18 can be used cookbook examples
@andys8
Copy link
Contributor Author

andys8 commented Aug 10, 2022

A bit of additional context: The first migrated react v18 example isn't working, and my assumption is it will start doing that after updating the package set.

It can be tested before and after with this link:

https://try.purescript.org/?github=JordanMartinez%2Fpurescript-cookbook%2Fmaster%2Frecipes%2FHelloReactHooks%2Fsrc%2FMain.purs&session=00809ee9-fae3-855c-7d22-570a92cbb7c2

Current State

image

andys8 added a commit to andys8/purescript-cookbook that referenced this pull request Aug 10, 2022
@JordanMartinez
Copy link
Contributor

I think the real culprit is the import map. See https://github.com/purescript/trypurescript/blob/master/client/public/frame.html#L28-L30

@andys8
Copy link
Contributor Author

andys8 commented Aug 10, 2022

Oha, I didn't know this is the way JS dependencies are provided. Pushed 257c1bf

I think the real culprit

I assume both are valid issues. JS needs to be updated (otherwise JS error at runtime) and PureScript bindings available (the latter currently fails at compile time).

@andys8 andys8 changed the title Update package-set to 0.15.4-20220808 Update package-set to 0.15.4-20220808 and react dependency Aug 11, 2022
@JordanMartinez
Copy link
Contributor

I still need to try this out locally to verify it works, but I don't see why it wouldn't.

Copy link
Contributor

@JordanMartinez JordanMartinez left a comment

Choose a reason for hiding this comment

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

When I build this locally, I get an error of Unknown module React.Basic.DOM.Client when I try to run the cookbook repo's HelloReactHooks example:

module HelloReactHooks.Main where

import Prelude

import Data.Maybe (Maybe(..))
import Effect (Effect)
import Effect.Exception (throw)
import React.Basic.DOM as R
import React.Basic.DOM.Client (createRoot, renderRoot)
import React.Basic.Hooks (Component, component)
import Web.DOM.NonElementParentNode (getElementById)
import Web.HTML (window)
import Web.HTML.HTMLDocument (toNonElementParentNode)
import Web.HTML.Window (document)

main :: Effect Unit
main = do
  doc <- document =<< window
  root <- getElementById "root" $ toNonElementParentNode doc
  case root of
    Nothing -> throw "Could not find root."
    Just container -> do
      reactRoot <- createRoot container
      app <- mkApp
      renderRoot reactRoot (app {})

mkApp :: Component {}
mkApp = do
  component "App" \_ -> React.do
    pure (R.text "Hello!")

I'm not sure why. I won't be able to look into this further today.

@andys8
Copy link
Contributor Author

andys8 commented Aug 12, 2022

Hi there, I tried it out locally, too. It took some time to install both the whole Haskell and PureScript world before being able to run it, but it was worth it.

It looks like another export for the subdirectory was missing to be able to execute the new react examples: react-dom/client. It's now added to the importmap: 823e716

Here is a screenshot of trypurescript working with the example in Chrome and Firefox.

image

@JordanMartinez However, I do not think this is the error you mentioned. "Unknown module React.Basic.DOM.Client" is the PureScript error that this PR should fix by updating the package set. I did not encounter it myself locally - even before the importmap fix. Instead - if I had to guess - I would double check the branch is correct, the build is done and spago's cache was updated. Let me know if you still encounter it.

@JordanMartinez
Copy link
Contributor

I think the issue was caused by me running the wrong npm commands. I was running npm run production:build && npm run serve. Notice how there's now npm run bundle in that, so I believe it was always bundling the old client.js code and never what I was working on. I should have been using npm run serve:production.

@JordanMartinez
Copy link
Contributor

I did not encounter it myself locally - even before the importmap fix. Instead - if I had to guess - I would double check the branch is correct, the build is done and spago's cache was updated. Let me know if you still encounter it.

This tells me it's likely something on my end. I'm nuking things and will see how things go from there.

@andys8
Copy link
Contributor Author

andys8 commented Aug 13, 2022

I should have been using npm run serve:production.

I think it has to be the dev version of commands to use the local setup. The production deployment wouldn't know of the new package set, right?

Use build:dev and base:dev if you are using a local Try PureScript server,
e.g. you followed the instructions in step 1.

Use build:production and base:production if you would like
to test the client against the production Try PureScript server.
npm run build:(dev|production)

@JordanMartinez
Copy link
Contributor

I should have been using npm run serve:production.

I think it has to be the dev version of commands to use the local setup. The production deployment wouldn't know of the new package set, right?

Yup!
Ok, got it working, and yes we do need the react-dom/client import mapping.

@@ -18,16 +18,17 @@
</script>
<!--
JSPM Generator Import Map
Edit URL: https://generator.jspm.io/#Y2NhYGCzD80rySzJSU1hSMpM183MK0lNTy1yMNQz0zM1ZEhJTc7MTczRyyp2MDTQM9YzZChKTUwu0U3Jz3UwNNcz0DPCENAvTi0qSy2CiMMUlZZmpjhYAA0wAgA6XlZ2dAA
Edit URL: https://generator.jspm.io/#ZcxBDkAwEEDRWQinMQwhlr2EA9BOZKQqqamtq5OIle1L/s8zgOIag4p6djDLUkpQXjgawh47AsdWtsnjehiqsUWCyJPV0u2boQEbrH9QWS8c9O8Hx5Pj61+ckjgzPOPmBszFaRmMAA
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there's a bug with jspm right now. This URL doesn't actually open anything for me. However, if I navigate to the above URL and then navigate to https://generator.jspm.io, then it shows what the URL should have displayed. 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems to work for me. 😅 Tested the link on mobile, different device.

Screenshot_2022-08-13-02-19-49-55_4d38fce200f96aeac5e860e739312e76

Copy link
Contributor

@JordanMartinez JordanMartinez left a comment

Choose a reason for hiding this comment

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

Thanks for all your work!

@JordanMartinez JordanMartinez merged commit 9116105 into purescript:master Aug 13, 2022
@JordanMartinez
Copy link
Contributor

Try PS was redeployed, but I'm getting this error when I run the HelloReactHooks example:

Uncaught TypeError: Error resolving module specifier “react-dom”. Relative module specifiers must start with “./”, “../” or “/”.

@JordanMartinez
Copy link
Contributor

Undoing #295 locally fixes the issue. Ugh...

@JordanMartinez
Copy link
Contributor

@andys8 Since I seem to have bad luck with my environment, can you confirm the above? Both that HelloReactHooks fails and that undoing shimMode: true makes it work again?

@JordanMartinez JordanMartinez mentioned this pull request Aug 13, 2022
4 tasks
andys8 added a commit to andys8/purescript-cookbook that referenced this pull request Aug 13, 2022
JordanMartinez pushed a commit to JordanMartinez/purescript-cookbook that referenced this pull request Aug 16, 2022
* Update package-set (same as try.purescript.org)

See <purescript/trypurescript#294>

* Update BookReactHooks

* Update ButtonsReactHooks

* Update CardsReactHooks

* Update CatGifsReactHooks

* Update ClockReactHooks

* Update ComponentsInputReactHooks

* Update ComponentsMultiTypeReactHooks

* Update ComponentsReactHooks

* Update DragAndDropReactHooks

* Update FileUploadReactHooks

* Update FormsReactHooks

* Update GroceriesReactHooks

* Update ImagePreviewsHalogenHooks

* Update NumbersReactHooks

* Update PositionsReactHooks

* Update RoutingHashReactHooks

* Update RoutingPushReactHooks

* Update ShapesReactHooks

* Update TextFieldsReactHooks

* Update TicTacToeReactHooks

* Update TimeReactHooks
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.

3 participants