-
Notifications
You must be signed in to change notification settings - Fork 99
SPARK-12: Move to monorepo setup by splitting Stacks Docs and Classic into NPM workspaces #1969
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
🦋 Changeset detectedLatest commit: 699adb6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Thanks @mukunku for making great progress on this. ❤️
I have left a few comments to address. The major one is about embracing the npm workspaces cli more to avoid having to cd into packages and changing our workflows more than necessary.
The rest of the stuff is pretty minor. I have not tested yet that the publishing workflow still work as expected. I will do that in a second review after the package.json files get adjusted.
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.
Thanks @mukunku
Notes from Giamir's second review pass (we can discuss and address those sync later):
- Update .gitattributes to point to new location of the screenshots
**/screenshots/**to make sure the files are still treated using git LFS and don’t blow up the repo - path in the
test.ymlworkflow for uploading the regression test results should be adjusted topackages/stacks-classic/screenshots - Maintain
npm starton the root package.json as a shortcut to start up the docs site quickly for development - Need to specify prettier config files also for the format script in stacks classic as well
"format": "prettier --write ./lib --config ../../.prettierrc --ignore-path ../../.prettierignore" - cannot run the visual test via
npm run test:visual -w packages/stacks-classic- need to specify the config file like we do in the GH workflow--config ./packages/stacks-classic/web-test-runner.config.mjs- A better solution would be actually to modify line 32 and 33 in the run-test-visual.ps1 to -v "${current_dir}/packages/stacks-classic/web-test-runner.config.mjs:/app/web-test-runner.config.mjs"-v "${current_dir}/packages/stacks-classic/web-test-runner.config.ci.mjs:/app/web-test-runner.config.ci.mjs" jquery,list.jsanddocsearch.jsshould move as dependencies of stacks-docs (be removed from root)- We need to add a changeset with a patch release for stacks-classic and ensure that the release process still run smoothly https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md#i-am-in-a-multi-package-repository-a-mono-repo
- NIT: for build, format and lint we could use
npm run <command> —workspaces —if-presentsyntax - NIT: we could add a
testscript in the root package.json in a similar fashion to what we do for lint, format and build already
|
Hey @giamir , just to follow up on our discussion today: I believe I addressed all the items you mentioned in your second review. Kindly take a look and let me know how the PR looks now 🙌🏼 I additionally created the changeset file now. I didn't see any workflow that runs until the PR is merged so I guess we'll know if it works after we merge? Finally, just wanted to share a recap of the It seems I broke the npm script locally while trying to get it to work in CI. As you mentioned when running visual tests locally the script wasn't picking up the web test runner configuration file. The main issue as discussed is caused by the relative node_module paths in web-test-runner.config.ci.mjs When we run a different test like a11y tests via: But when we run Lines 15 to 20 in 2942938
So to accommodate I had to mount the web-test-runner config files in sub-directories within the image so the paths align: Stacks/packages/stacks-classic/run-test-visual.ps1 Lines 32 to 33 in 699adb6
I gave the path a unique name so it's not confused with existing file paths. Consequently, I adjusted the path we pass in the test workflow: Stacks/.github/workflows/workflow.yml Line 42 in 699adb6
And locally:
|
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.
Thanks @mukunku. Let's merge this. 🎉
I additionally created the changeset file now. I didn't see any workflow that runs until the PR is merged so I guess we'll know if it works after we merge?
Yes, that's fair.
* SPARK-8: Fix Modal Focus Issue when using Keyboard (#1963) * fix reported focus issue * Also handle case where modal has no tabbable elements * fix webpack.config.js for windows * move if check up * Create loud-suits-stare.md * fix whitespace * revert html changes * revert harder * add tests * lint * chore(new-release) (#1964) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * [SPARK-6] - update the notice border color for dark mode and high contrast dark mode (#1965) * Correct readme * Don't run minicssextractplugin when running locally * Revert changes * update border colour for dark mode and high contrast dark mode * Create tasty-masks-pump.md * Minor notice border style refactor * Update visual regression test images --------- Co-authored-by: Dan Cormier <[email protected]> * chore(new-release) (#1966) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * build(deps-dev): bump webpack from 5.99.9 to 5.101.2 (#1967) Bumps [webpack](https://github.com/webpack/webpack) from 5.99.9 to 5.101.2. - [Release notes](https://github.com/webpack/webpack/releases) - [Commits](webpack/webpack@v5.99.9...v5.101.2) --- updated-dependencies: - dependency-name: webpack dependency-version: 5.101.2 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * SPARK-12: Move to monorepo setup by splitting Stacks Docs and Classic into NPM workspaces (#1969) * mention why we are doing this as part of project SHINE * move folders * update paths * fix docs site and visual tests script * fix a11y and unit test scripts and tweak visual regression threshold * fix workflow tests * revert ci import paths to fix tests * try older image * fix visual ci tests? * mark stacks-docs not publishable * adjust adr based on pr feedback * adjust names * address PR feedback * add missing workspace param * switch back to old script * fix netlify and a11y * fix path * try fix workflows * fix harder * PR feedback * pr feedback round 2 * move some more packages around * Create eighty-pillows-relate.md * chore(screenshots): adjust gitignore patterns excluding failed images (#1972) * chore(monorepo): follow-up tweaks post monorepo changes (#1974) * chore(git-lfs): ensure visual test baseline images are tracked by git lfs (#1978) * Fix aria-current attribute placement in sidebar widget navigation examples (#1970) * chore(new-release) (#1979) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * build(deps-dev): bump vite from 7.1.3 to 7.1.5 (#1980) Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 7.1.3 to 7.1.5. - [Release notes](https://github.com/vitejs/vite/releases) - [Changelog](https://github.com/vitejs/vite/blob/main/packages/vite/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite/commits/v7.1.5/packages/vite) --- updated-dependencies: - dependency-name: vite dependency-version: 7.1.5 dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: svc_tooling <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Tavian Taylor <[email protected]> Co-authored-by: Dan Cormier <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Giamir Buoncristiani <[email protected]> Co-authored-by: qheaden-stack <[email protected]>
* SPARK-8: Fix Modal Focus Issue when using Keyboard (#1963) * fix reported focus issue * Also handle case where modal has no tabbable elements * fix webpack.config.js for windows * move if check up * Create loud-suits-stare.md * fix whitespace * revert html changes * revert harder * add tests * lint * chore(new-release) (#1964) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * [SPARK-6] - update the notice border color for dark mode and high contrast dark mode (#1965) * Correct readme * Don't run minicssextractplugin when running locally * Revert changes * update border colour for dark mode and high contrast dark mode * Create tasty-masks-pump.md * Minor notice border style refactor * Update visual regression test images --------- Co-authored-by: Dan Cormier <[email protected]> * chore(new-release) (#1966) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * build(deps-dev): bump webpack from 5.99.9 to 5.101.2 (#1967) Bumps [webpack](https://github.com/webpack/webpack) from 5.99.9 to 5.101.2. - [Release notes](https://github.com/webpack/webpack/releases) - [Commits](webpack/webpack@v5.99.9...v5.101.2) --- updated-dependencies: - dependency-name: webpack dependency-version: 5.101.2 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * SPARK-12: Move to monorepo setup by splitting Stacks Docs and Classic into NPM workspaces (#1969) * mention why we are doing this as part of project SHINE * move folders * update paths * fix docs site and visual tests script * fix a11y and unit test scripts and tweak visual regression threshold * fix workflow tests * revert ci import paths to fix tests * try older image * fix visual ci tests? * mark stacks-docs not publishable * adjust adr based on pr feedback * adjust names * address PR feedback * add missing workspace param * switch back to old script * fix netlify and a11y * fix path * try fix workflows * fix harder * PR feedback * pr feedback round 2 * move some more packages around * Create eighty-pillows-relate.md * chore(screenshots): adjust gitignore patterns excluding failed images (#1972) * chore(monorepo): follow-up tweaks post monorepo changes (#1974) * chore(git-lfs): ensure visual test baseline images are tracked by git lfs (#1978) * Fix aria-current attribute placement in sidebar widget navigation examples (#1970) * chore(new-release) (#1979) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * build(deps-dev): bump vite from 7.1.3 to 7.1.5 (#1980) Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 7.1.3 to 7.1.5. - [Release notes](https://github.com/vitejs/vite/releases) - [Changelog](https://github.com/vitejs/vite/blob/main/packages/vite/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite/commits/v7.1.5/packages/vite) --- updated-dependencies: - dependency-name: vite dependency-version: 7.1.5 dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(docs): add readme for each workspace (#1982) * add readme's for each package * Create six-buckets-grab.md * adjust stacks classic readme a bit * don't publish changeset for private package stacks-docs (#1983) * chore(new-release) (#1985) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: svc_tooling <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Tavian Taylor <[email protected]> Co-authored-by: Dan Cormier <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Giamir Buoncristiani <[email protected]> Co-authored-by: qheaden-stack <[email protected]>
Summary
This is for SPARK-12 and is being done in preparation for project SHINE.
This PR transitions the stacks classic and stacks docs projects into the npm workspaces structure. This allows several different packages to be maintained in a single monorepo, which we will need once we start bringing stacks svelte, etc into this repo.
How to Test
Testing is straightforward: Everything should work the same as it did before.
How to Review (To be updated)
Most of the file changes in this PR are from cheese being moved around. Mainly moving the
docsandlibfolders into new npm workspaces. Here are a list of file changes that are not part of this simple move; I left comments on the files themselves to share my thoughts and rationale for the changes.