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

Refine package depencencies #200

Closed
4 of 5 tasks
techniq opened this issue Jan 8, 2024 · 4 comments
Closed
4 of 5 tasks

Refine package depencencies #200

techniq opened this issue Jan 8, 2024 · 4 comments
Assignees
Milestone

Comments

@techniq
Copy link
Owner

techniq commented Jan 8, 2024

Need to think how this might impact LayerChart (if at all)

Update Svelte UX

  • Move prism* from peer to dev
  • Move fortawesome/* from peer to dev
  • Remove sveld from peer (already dev)
    • Should this be moved from devDepencies to dependencies due to the vite plugin which is used in LayerChart?

Update LayerChart

  • Add prismjs and prism-svelte as dev
  • sveld? (see above)
@techniq techniq added this to the v1 milestone Jan 8, 2024
@jycouet jycouet self-assigned this Jan 8, 2024
@jycouet jycouet mentioned this issue Jan 8, 2024
7 tasks
@jycouet
Copy link
Collaborator

jycouet commented Jan 8, 2024

Actually, you already did it in layerChart 4 days ago: https://github.com/techniq/layerchart/blob/dd55df13282f33665f38a8cd1482678ec9b961b3/packages/layerchart/package.json#L53-L55

PR open for svelte-ux 👍

@techniq
Copy link
Owner Author

techniq commented Jan 8, 2024

Actually, you already did it in layerChart 4 days ago: https://github.com/techniq/layerchart/blob/dd55df13282f33665f38a8cd1482678ec9b961b3/packages/layerchart/package.json#L53-L55

PR open for svelte-ux 👍

Oh yeah, when I removed the package-lock.json file to fix the Cloudflare build, these kind of issues showed up :). Thanks!

Once we finish PR's #191 and #192 and merge into next branch/PR, I publish a new svelte-ux@next release and update the LayerChart design tokens PR / next package and we can see if we need to do anything else.

@techniq
Copy link
Owner Author

techniq commented Jan 10, 2024

@jycouet Updated LayerChart to the latest Svelte UX next.9 release which includes these changes. I was a bit surprised prismjs and sveld didn't complain when LayerChart is built now, but I'm just realizing we added those as dependencies and not devDependencies.

Technically that's right if using the vite plugin (sveld) or the Code/Preview components (prism). These both only apply to LayerChart for it's docs, so maybe we're OK because we...

  • Don't top-level/barrel export Code or Preview. LayerChart currently uses their own copy
  • Don't top-level/barrel export any of the plugins directly, it's setup as their own export

So maybe all is good, I just wanted to make sure we don't inflate the bundle with these.

Speaking of, there is a nice Table excel export util that I currently do not expose because await import('exceljs') didn't work within that util the last time I tried (I only want to pull that large exceljs dep when export is requested. Will need to see if vite supports this now (or whatever caused it to error last time).

@techniq
Copy link
Owner Author

techniq commented Feb 14, 2024

done

@techniq techniq closed this as completed Feb 14, 2024
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

No branches or pull requests

2 participants