Skip to content

Preload All Scripts #9

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

Merged
merged 6 commits into from
Nov 3, 2020
Merged

Preload All Scripts #9

merged 6 commits into from
Nov 3, 2020

Conversation

shadowtime2000
Copy link
Member

Should resolve #7.

@shadowtime2000 shadowtime2000 requested a review from ije as a code owner November 3, 2020 01:40
@vercel
Copy link

vercel bot commented Nov 3, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/postui/alephjs-hello-world/obldp1oi8
✅ Preview: https://alephjs-hello-world-git-preload-files.postui.vercel.app

@ije
Copy link
Member

ije commented Nov 3, 2020

  • thanks, but consider the createHtml is a help function, please add the preload scripts in project.ts
  • to keep code style, please use vscode default formater: always use ' for plain string and no ; as line end

@ije
Copy link
Member

ije commented Nov 3, 2020

in fact all the preload-able scripts will pre-import in main.js(please check it in chrome) and main.js is preloaded, i am wondering maybe no preload scripts needed (should test it)

@ije
Copy link
Member

ije commented Nov 3, 2020

and the preload script should be <link rel="modulepreload" src="..." /> since it's an esm script

@shadowtime2000
Copy link
Member Author

in fact all the preload-able scripts will pre-import in main.js(please check it in chrome) and main.js is preloaded, i am wonder maybe no preload scripts needed (should test it)

@ije I think you are still going to need to preload those others scripts. Check this snip from the Google Lighthouse:
image

@ije
Copy link
Member

ije commented Nov 3, 2020

i see, thanks, do you have a spare time to check the project.ts(it's a bit large)? the preload scripts shoud be added in here not html.ts, or i will finish it later.

@ije
Copy link
Member

ije commented Nov 3, 2020

add preload scripts with createHtml function:

createHtml({
    ...
    scripts: [
        { src: '...', type: 'module', preload: true },
        ...
    ],
    ...
})

@shadowtime2000
Copy link
Member Author

I will take a look at it.

@ije
Copy link
Member

ije commented Nov 3, 2020

cool!

@shadowtime2000
Copy link
Member Author

@ije I moved the changes to project.ts.

@ije
Copy link
Member

ije commented Nov 3, 2020

@shadowtime2000 great!! i would create a function like:

function getPreloadScripts(baseUrl: string) {
    const scripts = [
         '-/deno.land/x/aleph/aleph.js',
        '-/deno.land/x/aleph/context.js',
        '-/deno.land/x/aleph/error.js',
        '-/deno.land/x/aleph/events.js',
        '-/deno.land/x/aleph/routing.js,',
        '-/deno.land/x/aleph/util.js'
    ]
    return scripts.map(src => ({ src: `${baseUrl}${src}`, type: 'module', preload: true }))
}

then inject it to all the createHtml calls:

createHtml({
    ...
    scripts: [
        ...
        ...getPreloadScripts(baseUrl),
        ...
    ],
    ...
})

then i will marge this request, thanks

@shadowtime2000
Copy link
Member Author

@ije Done.

@ije
Copy link
Member

ije commented Nov 3, 2020

LGTM; thanks

@ije ije merged commit c662de1 into alephjs:master Nov 3, 2020
@shadowtime2000 shadowtime2000 deleted the preload-files branch November 3, 2020 17:54
@ije ije mentioned this pull request Nov 3, 2020
30 tasks
mohsenkhanpour pushed a commit to mohsenkhanpour/aleph.js that referenced this pull request Jan 29, 2021
Update hmr-with-fast-refresh.md
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.

Add Preload Link Tags To Head
2 participants