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

Move to Svelte Kit #75

Merged
merged 20 commits into from
Nov 26, 2021
Merged

Move to Svelte Kit #75

merged 20 commits into from
Nov 26, 2021

Conversation

designorant
Copy link

@designorant designorant commented Nov 24, 2021

This was fun. Came across this fun issue, found a bug in Svelte, a bug in ONS Design System, almost lost my sanity on this bug / lack of feature thanks to the magical "client side routing" that intercepts everything, and ended up using a nasty workaround that adds a <script /> tag on each route, literally.

The SSR isn't enabled just yet, need some fixes for accessing Window object, but will get there.

Cut into logical chunks for easy merging, and rebased on master; all seems fine but let's wait for the Netlify bot to confirm. 🤞

Edit: Netlify bot disagrees even though yarn preview seems to work just fine locally.

@netlify
Copy link

netlify bot commented Nov 24, 2021

✔️ Deploy Preview for ons-data-map ready!

🔨 Explore the source changes: d42d64b

🔍 Inspect the deploy log: https://app.netlify.com/sites/ons-data-map/deploys/619f89332c080c00089b2b5c

😎 Browse the preview: https://deploy-preview-75--ons-data-map.netlify.app

@designorant
Copy link
Author

Looks like it reads the config file just fine:

6:41:19 PM: Different publish path detected, going to use the one specified in the Netlify configuration file: 'build' versus 'public' in the Netlify UI
6:41:19 PM: Different build command detected, going to use the one specified in the Netlify configuration file: 'yarn build' versus 'npm run build' in the Netlify UI

but chokes on the D3 packages:

6:42:07 PM: > Using @sveltejs/adapter-netlify
6:42:08 PM:  > .svelte-kit/output/server/chunks/original-5947e2b1.js:3:7: warning: Ignoring this import because "node_modules/d3-scale/dist/d3-scale.js" was marked as having no side effects
6:42:08 PM:      3 │ import "d3-scale";
6:42:08 PM:        ╵        ~~~~~~~~~~
6:42:08 PM:    node_modules/d3-scale/package.json:35:2: note: "sideEffects" is false in the enclosing "package.json" file
6:42:08 PM:     35 │   "sideEffects": false,
6:42:08 PM:        ╵   ~~~~~~~~~~~~~
6:42:08 PM:  > .svelte-kit/output/server/chunks/original-5947e2b1.js:4:7: warning: Ignoring this import because "node_modules/d3-array/dist/d3-array.js" was marked as having no side effects
6:42:08 PM:      4 │ import "d3-array";
6:42:08 PM:        ╵        ~~~~~~~~~~
6:42:08 PM:    node_modules/d3-array/package.json:39:2: note: "sideEffects" is false in the enclosing "package.json" file
6:42:08 PM:     39 │   "sideEffects": false,
6:42:08 PM:        ╵   ~~~~~~~~~~~~~
6:42:08 PM:  > .svelte-kit/output/server/chunks/original-5947e2b1.js:5:7: warning: Ignoring this import because "node_modules/d3-shape/dist/d3-shape.js" was marked as having no side effects
6:42:08 PM:      5 │ import "d3-shape";
6:42:08 PM:        ╵        ~~~~~~~~~~
6:42:08 PM:    node_modules/d3-shape/package.json:40:2: note: "sideEffects" is false in the enclosing "package.json" file
6:42:08 PM:     40 │   "sideEffects": false,
6:42:08 PM:        ╵   ~~~~~~~~~~~~~
6:42:08 PM:   ✔ done

Google-fu give some pointers.

@ONSdigital ONSdigital deleted a comment from netlify bot Nov 25, 2021
@designorant designorant force-pushed the feature/svelte-kit branch 2 times, most recently from 02b89a2 to 0560922 Compare November 25, 2021 02:37
@designorant
Copy link
Author

It's not a job, it's a journey. Ready for review.

Some background to the latest fixes and workarounds:

Also removed Storybook for quicker deploy times. Locally this thing flies compared to the original now.

Copy link

@vmilitaru vmilitaru left a comment

Choose a reason for hiding this comment

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

Today, makes more sense to me the changes that you've made to implement the svelte kit and I would honestly like to see it merged in so that I can play with it .

@designorant designorant merged commit 9869e40 into master Nov 26, 2021
@designorant designorant deleted the feature/svelte-kit branch November 26, 2021 11:24
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

2 participants