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.

Sign up for GitHub

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

Jump to bottom

add sandpack to ajaxapp #1423

Merged
merged 3 commits into from
Nov 5, 2022
Merged

add sandpack to ajaxapp #1423

merged 3 commits into from
Nov 5, 2022

Conversation

lacolaco
Copy link
Collaborator

@lacolaco lacolaco commented Jun 1, 2022

@bot-user
Copy link

bot-user commented Jun 1, 2022 •
edited
Loading

✅ Deploy Preview for js-primer ready!

Name Link
🔨 Latest commit 6920ad3
🔍 Latest deploy log https://app.netlify.com/sites/js-primer/deploys/6366656c5ebaaa0009eefc90
😎 Deploy Preview https://deploy-preview-1423--js-primer.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@lacolaco
Copy link
Collaborator Author

lacolaco commented Jun 1, 2022

@azu https://deploy-preview-1423--js-primer.netlify.app/use-case/ajaxapp/http/
ここで index.jsで宣言してるはずの関数がsandboxだと見つからないみたいなんですが、原因わかりますか？

https://github.com/asciidwango/js-primer/pull/1423/files#diff-18c65cac366ee2f24d884fbc5d7054f4f59c804daa38e5105de49df07e711487

@azu azu marked this pull request as ready for review June 1, 2022 03:17
@azu
Copy link
Collaborator

azu commented Jun 1, 2022

みてみる

@azu
Copy link
Collaborator

azu commented Jun 1, 2022 •
edited
Loading

index.jsは読み込まれてるけど、globalなfetchUserInfoはないみたいな状態なんだな。
sandpackでは動かなくて、codesandboxでは動くみたいなやつか
https://codesandbox.io/s/4wbbrn?file=/index.html&from-sandpack=true

function fetchUserInfo(userId) {
    fetch(`https://api.github.com/users/${encodeURIComponent(userId)}`)
        .then(response => {
            console.log(response.status);
            // エラーレスポンスが返されたことを検知する
            if (!response.ok) {
                console.error("エラーレスポンス", response);
            } else {
                return response.json().then(userInfo => {
                    console.log(userInfo);
                });
            }
        }).catch(error => {
            console.error(error);
        });
}

globalThis.fetchUserInfo = fetchUserInfo;

みたいにすれば、動くのはわかったけど、index.jsを実行する時に何かの関数に包まれててglobalに見えてないのかなー。
ちょっと後で調べてみる

sandpack こういう細かい挙動の違いがあって、それを吸収ワークアラウンドを追加する必要があるのかなー

js-primer/source/use-case/todoapp/update-delete/README.md

Line 203 in 627de17

"appendCode": "// 次の行は本編とは無関係のコードなので無視してください\nconst link = document.createElement('link');link.rel='stylesheet';link.href='https://jsprimer.net/use-case/todoapp/final/final/index.css';document.head.append(link);\n",

@azu
Copy link
Collaborator

azu commented Jun 1, 2022 •
edited
Loading

externalResourcesを使うことで、ここに指定したリソースは、<head> にinjectされる形になって、
<script src> で指定したのと大体同じ挙動になりそう。(HTMLに書かれてる <script src=index.js>はentryポイントをずらすことで何もしない形にしてる。このentryはvanillaだと何もしない。parcelとか指定するとindex.htmlをentryにしてbundleするみたいな意味合いになる)

https://sandpack.codesandbox.io/docs/getting-started/custom-content#static-external-resources

<!-- sandpack:{
  "files": {
    "/index.js": {
      "path": "src/index.js"
    },
    "/index.html": {
      "path": "src/index.html",
      "active": true
    },
    "/src/index.js": {
      "code": "/* このファイルは本編とは無関係のファイルなので無視してください。 本編のindex.jsは一つ上のディレクトリにあります */",
      "hidden": true
    }
  },
  "entry": "/index.html",
  "main": "/index.html",
  "environment": "static",
  "template": "vanilla",
  "options": {
    "showLineNumbers": true,
    "editorHeight": 550,
    "externalResources": ["https://jsprimer.net/use-case/ajaxapp/http/src/index.js"]
  },
  "honkitSettings": {
    "isOpen": true,
    "hideExitButton": true
  }
} -->

欠点としては、externalResourcesはHTTPなリソースしか指定できないので、デプロイされるまで、
sandpack上は古いものが見えてしまうとう問題がある感じ。
Codesandbox上は index.htmlとindex.js で普通に動くのでちゃんとしてる…

(todoappでcssをworkaroundしてるやつはexternalResourcesを使った方が綺麗なので、移行しておこ)

@azu azu mentioned this pull request Jun 1, 2022
Closed
lacolaco and others added 2 commits November 5, 2022 21:06
@azu azu force-pushed the add-sandpack-ajaxapp branch from 5c02068 to 36fb3c5 Compare November 5, 2022 13:25
@azu
Copy link
Collaborator

azu commented Nov 5, 2022

  • コンソールの対応
  • sandpackのプレビューで動くように

sandpackがvanillaのenviromentだとscript srcでのindex.jsを読み込めない問題がある。
clickしたときにmain() とかが見つからない問題が起きる

これを回避するために、任意のURLをheadにinjectするexternalResourcesを使って回避してる。

"externalResources": ["https://jsprimer.net/use-case/ajaxapp/http/src/index.js"],

これでglobalにjsを入れられるので main() が見つけられる。

script type=moduleとかをつかったコードとかなら、enviroment parcelにすればbundleされるのでこのハックをしなくてもいける気がする。

@azu
最後は締める言葉にする
6920ad3 
Signed-off-by: azu <azuciao@gmail.com>
@azu
Copy link
Collaborator

azu commented Nov 5, 2022

image

それぞれの最後にURLとプレビューを追加した。

URLが /src でいいのかはちょっと気になるので、アジェンダに入れた。
紙面的には何かクッション挟める仕組みするのがいいんだろかとか src/ というURLがどうなのか

azu
azu reviewed Nov 5, 2022
View reviewed changes
source/_layouts/website/page.html
Comment on lines -89 to +90
width: calc(100% + 240px);
margin: 0 0 1em -120px;
width: calc(100% + 180px);
margin: 0 0 1em -90px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

ちょっとprev/nextボタンと被り気味だった。
これHonKit/GitBookのデフォルトテーマのボタンが良くない。

azu
azu reviewed Nov 5, 2022
View reviewed changes
source/use-case/ajaxapp/entrypoint/README.md
"options": {
"showLineNumbers": true,
"editorHeight": 550,
"externalResources": ["https://jsprimer.net/use-case/ajaxapp/entrypoint/src/index.js"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

https://github.com/honkit/honkit-plugin-sandpack#note で書いてるハック的な。
sandpackのvanilla envでは、script srcとかがちゃんと読めない。
parcel envでは、entry(index.html)をbundleするのでいけるというものらしい。
vanillaのままやるにはexternalResourcesを使うしかない。

@azu azu merged commit 2dd632e into master Nov 5, 2022
@azu azu deleted the add-sandpack-ajaxapp branch November 5, 2022 13:36
@azu
Copy link
Collaborator

azu commented Nov 5, 2022

とりあえず動くようになったので、マージします

@azu
Copy link
Collaborator

azu commented Nov 5, 2022

externalResources だと編集ができないことに気づいた(読んでいるURLはサーバにあるため)

This was referenced Nov 5, 2022
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@azu azu azu left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lacolaco @bot-user @azu