Closed milafrerichs closed 3 years ago
Oh and another thing - please run npm run prettier
(in the presenter package) to process all the source files with Prettier. Thanks!
Surfacing some Slack discussion:
Re: build sizes, yes - I wonder if the +136.73 KB is due to the Svelte compiler. It's hard to tell from these npm run build
reports. The best way would be to create the production build, host it locally, then open up a viz page with the editor open, and see in the Network tab how many KB of JavaScript is fetched in total. The number can be compared across branches.
Re: comment in more detail - I may be wrong about it. My thinking was that the dynamic import would trigger even when no .svelte
files are present, however, on closer reading, I see that transform short circuits if the file does not end with .svelte
, so it seems it does the right thing actually right now. The "right thing" is:
App.svelte
, the Svelte compiler dynamic import should not trigger.App.svelte
, the Svelte compiler dynamic import should trigger.It should be possible to test this out easily if I put this on staging... We could just look at the Network tab total JS for two different vizzes, one with App.svelte and one without. There should be more JS loaded if there is App.svelte.
Deployed this branch to Staging just now: https://staging.vizhub.com/
Trying it out in the editor https://staging.vizhub.com/curran/2db71d53e8a44ac5a3056638ef5cfd12?edit=files&file=App.svelte&line
It appears that bundle.js
is not generated, after introducing App.svelte
via the editor. There may be some logic that checks for index.js
to enable Rollup. That may need to be expanded to detect App.svelte
.
See packages/neoFrontend/src/pages/VizPage/RunContext/updateBundleIfNeeded.js
@milafrerichs I made a PR that, when merged, will go into this branch and enable Rollup when App.svelte
is present. Please review and merge into here if you approve. Thanks!
It appears that
bundle.js
is not generated, after introducingApp.svelte
via the editor. There may be some logic that checks forindex.js
to enable Rollup. That may need to be expanded to detectApp.svelte
.See
packages/neoFrontend/src/pages/VizPage/RunContext/updateBundleIfNeeded.js
You always need an index.js
anyway for svelte.
Take a look at their starter package: https://github.com/sveltejs/template
Maybe we can generate that for our users in the long run.
If you include the following it will work:
import App from './App.svelte';
const app = new App({ target: document.body, });
export default app;
see this example: https://staging.vizhub.com/milafrerichs/700cdab4dbff41c5a07038c4597c1db3?edit=files&file=bundle.js&line
Oh I see!!! Thanks.
So that PR I made just now is not valid actually... Closing.
Getting there! Now I see index.js:3 Uncaught ReferenceError: internal is not defined
.
Getting there! Now I see
index.js:3 Uncaught ReferenceError: internal is not defined
.
yeah, saw that as well. I assume it's trying to use svelte which is actually not there yet ;) The imports do not work. this was just to get it to compile into js which it does :)
Maybe we need this, or something like this https://unpkg.com/svelte@3.0.1/internal.js
Possibly a good idea from Slack discussion:
globals: { ...vizhubLibraries, 'svelte': 'svelte', 'svelte/internal': 'svelte'},
Not sure where that came from, but maybe makes sense. We need a svelte
global somehow, not sure where to get that.
it works 🎉
Looking into this failing test:
1) Presenters
Svelte
should bundle files using Rollup:
Error: 'SvelteComponent' is not exported by https://unpkg.com/svelte@3.31.0/internal/index.mjs, imported by App.svelte
Confirming that the tests pass on the previous commit f70405414f2938da6f3b0c9b6b6645f67cde0da2
WIP and POC of getting rollup to compile svelte files the tests is failing to show the output