-
Notifications
You must be signed in to change notification settings - Fork 100
SPARK-48: Publish Stacks-Svelte #1991
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
Conversation
in this commit I have done the following: - ensure that the correct typescript config is picked up by the svelte config (and consequentially vite) - ensure that svelte-sonnet does not get prebundled by esbuild (this was causing issues for the tests) - Keep peer dependencies for stacks svelte marked (we cannot put * because that would indicate that consumers can install any svelte or stacks classic version which is not the case) - Adjust stacks svelte web test runner config to point at the css bundle generated in stacks classic and also ensure that a fresh build of stacks classic is run before running the tests (I also removed the reference to stacks.js because we don't depend on that in stacks-svelte) - IMPORTANT: Ensure that storybook point at our stacks.less bundle in the preview.ts file. This has the benefit that whatever styles we change in stacks classic it will be hot reloaded in storybook (this is pretty much the whole point why we switch to monorepo - make a change in stacks classic and see it reflected in stacks svelte in a matter of milliseconds - pretty cool :) ) - There are 2 unit tests failing for the Select component - I have not looked too much into it because the component need to be migrated to use the new Svelte 5 runes API. Before spending too much time trying to debug the tests I would rather go ahead and migrate it. It should be rather simple.
🦋 Changeset detectedLatest commit: 0aa46de The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
❌ Deploy Preview for stacks failed. Why did it fail? →
|
✅ Deploy Preview for sal-docs canceled.
|
import "@stackoverflow/stacks/lib/index"; | ||
import "../less/stacks-documentation.less"; | ||
import "./controllers/docs-resizer"; | ||
import * as Stacks from "../../../stacks-classic/lib/index"; | ||
import * as Stacks from "@stackoverflow/stacks/lib/index"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
npm creates symbolic links of local peer dependencies inside node_modules
so it's possible to reference the files as an npm package. Thought that was cool so changed these imports to this. Let me know if we think relative paths were better here.
✅ Deploy Preview for stacks-svelte ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for stacks ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
@@ -0,0 +1,26 @@ | |||
import type { Preview } from "@storybook/svelte"; | |||
import "@stackoverflow/stacks/lib/stacks.less"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above:
npm creates symbolic links of local peer dependencies inside node_modules so it's possible to reference the files as an npm package. Thought that was cool so changed these imports to this. Let me know if we think relative paths were better here.
@@ -0,0 +1,43 @@ | |||
{ | |||
"name": "@stackoverflow/stacks-svelte", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed the package from @stackeng/stacks-svelte
to @stackoverflow/stacks-svelte
so it matches the org name for stacks classic. We can update internal consumers to the new package in separate PR's.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work with this @mukunku! I have one minor suggestion but nothing so significant that see it unchanged would be a real issue.
Context for @giamir / @ttaylor-stackI've reviewed this PR and found it to be pretty solid. The only necessary further context I think is necessary is the follow up tasks to happen post-merge:
|
Summary
This PR exposes our internal Svelte implementation of the Stacks design library publicly by adding it to the monorepo.
How to Test
stacks-svelte
workspace to confirm the PR is good.production
once this PR is merged and releases are performed.How to Review
Below are the most important files to checkout as part of reviewing this PR: