feat: Added NPM publish workflow for ADS package #37787
Conversation
WalkthroughThis pull request introduces a new GitHub Actions workflow file, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (1)
.github/workflows/npm-publish.yml (1)
3-9: Consider adding specific file patterns to the path filter.The current path filter might trigger unnecessarily for non-source files. Consider being more specific:
paths: - "app/client/packages/design-system/ads/**" + - "app/client/packages/design-system/ads/package.json" + - "app/client/packages/design-system/ads/src/**"🧰 Tools
🪛 yamllint (1.35.1)
[error] 9-9: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/npm-publish.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/npm-publish.yml
18-18: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
22-22: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
28-28: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
34-34: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 yamllint (1.35.1)
.github/workflows/npm-publish.yml
[error] 9-9: trailing spaces
(trailing-spaces)
[error] 19-19: trailing spaces
(trailing-spaces)
[error] 25-25: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 44-44: trailing spaces
(trailing-spaces)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 59-59: trailing spaces
(trailing-spaces)
[error] 74-74: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
.github/workflows/npm-publish.yml (1)
1-74: Verify workflow uniqueness.
Let's ensure this workflow doesn't conflict with existing npm publish workflows.
✅ Verification successful
Workflow is unique for its purpose
The npm-publish.yml workflow is the only one handling NPM package publishing for the design system. While there are other release-related workflows, they serve different purposes (Docker images, Helm charts, etc.). The workflow is properly scoped to only trigger on changes to "app/client/packages/design-system/ads/**" in the release branch.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other npm publish workflows
echo "Checking for existing npm publish workflows..."
fd -e yml -e yaml . .github/workflows/ -x grep -l "npm.*publish" {}
Length of output: 174
Script:
#!/bin/bash
# Let's try a different approach to find npm publish related workflows
# First, let's list all workflow files
echo "Workflow files:"
fd -e yml -e yaml . .github/workflows/
# Then, let's examine their content for npm publish related patterns
echo -e "\nChecking workflow contents:"
fd -e yml -e yaml . .github/workflows/ -x cat {} | grep -i "npm\|publish\|release" -B2 -A2
Length of output: 22195
🧰 Tools
🪛 actionlint (1.7.4)
18-18: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
22-22: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
28-28: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
34-34: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 yamllint (1.35.1)
[error] 9-9: trailing spaces
(trailing-spaces)
[error] 19-19: trailing spaces
(trailing-spaces)
[error] 25-25: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 44-44: trailing spaces
(trailing-spaces)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 59-59: trailing spaces
(trailing-spaces)
[error] 74-74: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/npm-publish.yml
Outdated
| - name: Setup Node js version | ||
| uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: 16.x |
There was a problem hiding this comment.
For appsmith we have node of version ^20.11.1 restriction, so you may want to use it here as well.
There was a problem hiding this comment.
We usually do it like this
uses: actions/setup-node@v4
with:
node-version-file: app/client/package.json
| commit: "ci(changesets): version packages" | ||
| title: "ci(changesets): version packages" | ||
| env: | ||
| NPM_TOKEN: ${{ secrets.NPM_TOKEN }} |
There was a problem hiding this comment.
Should it be APPSMITH_NPM_PUBLISH_KEY or NPM_TOKEN? Because in npm config you've used APPSMITH_NPM_PUBLISH_KEY. If both exist, then just pick a single key for consistence.
.github/workflows/npm-publish.yml
Outdated
| - name: Setup Node js version | ||
| uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: 16.x |
There was a problem hiding this comment.
We usually do it like this
uses: actions/setup-node@v4
with:
node-version-file: app/client/package.json
.github/workflows/npm-publish.yml
Outdated
| git config user.email github-actions@github.com | ||
|
|
||
| # Configure npm | ||
| - name: Configure npm |
There was a problem hiding this comment.
Why do we need this step?
There was a problem hiding this comment.
This is to authenticate with NPM key
Co-authored-by: Valera Melnikov <valera@appsmith.com>
Co-authored-by: Valera Melnikov <valera@appsmith.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
.github/workflows/npm-publish.yml (1)
3-9: Add package.json to path filters.Consider including package.json changes in the path filters to ensure version updates trigger the workflow:
paths: - "app/client/packages/design-system/ads/**" + - "app/client/packages/design-system/ads/package.json"🧰 Tools
🪛 yamllint (1.35.1)
[error] 9-9: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/npm-publish.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/npm-publish.yml
18-18: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
22-22: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
28-28: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 yamllint (1.35.1)
.github/workflows/npm-publish.yml
[error] 9-9: trailing spaces
(trailing-spaces)
[error] 19-19: trailing spaces
(trailing-spaces)
[error] 25-25: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 44-44: trailing spaces
(trailing-spaces)
[error] 50-50: trailing spaces
(trailing-spaces)
[error] 59-59: trailing spaces
(trailing-spaces)
[error] 74-74: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (2)
.github/workflows/npm-publish.yml (2)
27-31: Use node-version-file instead of hardcoded version.
As suggested in previous comments, use the version specified in package.json:
uses: actions/setup-node@v4
with:
- node-version: 16.x
+ node-version-file: app/client/package.json🧰 Tools
🪛 actionlint (1.7.4)
28-28: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 yamllint (1.35.1)
[error] 31-31: trailing spaces
(trailing-spaces)
51-59:
Standardize NPM token usage and add token verification.
- Use consistent token name across steps
- Add a token verification step before publishing
-NPM_TOKEN: ${{ secrets.APPSMITH_NPM_PUBLISH_KEY }}
+NPM_TOKEN: ${{ secrets.NPM_TOKEN }}Add before the publish step:
- name: Verify NPM token
run: npm whoami
env:
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}Also applies to: 72-74
🧰 Tools
🪛 yamllint (1.35.1)
[error] 59-59: trailing spaces
(trailing-spaces)
| # Create PR & publish to NPM | ||
| - name: Create release Pull Request or publish to NPM | ||
| id: changesets | ||
| uses: changesets/action@v1 | ||
| with: | ||
| publish: yarn release | ||
| commit: "ci(changesets): version packages" | ||
| title: "ci(changesets): version packages" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add version validation before publishing.
Add a step to validate package version before publishing to prevent accidental releases:
- name: Validate version
run: |
CURRENT_VERSION=$(node -p "require('./app/client/packages/design-system/ads/package.json').version")
NPM_VERSION=$(npm view @design-system/ads version 2>/dev/null || echo "0.0.0")
if [ "$(printf '%s\n' "$NPM_VERSION" "$CURRENT_VERSION" | sort -V | head -n1)" = "$CURRENT_VERSION" ]; then
echo "Error: New version ($CURRENT_VERSION) must be greater than published version ($NPM_VERSION)"
exit 1
fi
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
.github/workflows/npm-publish.yml (1)
5-6: Consider more specific path patterns.The current path pattern might trigger unnecessarily for non-package files. Consider being more specific:
- - "app/client/packages/design-system/ads/**" + - "app/client/packages/design-system/ads/package.json" + - "app/client/packages/design-system/ads/src/**" + - "app/client/packages/design-system/ads/dist/**"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/npm-publish.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/npm-publish.yml
21-21: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
🪛 yamllint (1.35.1)
.github/workflows/npm-publish.yml
[error] 9-9: trailing spaces
(trailing-spaces)
[error] 22-22: trailing spaces
(trailing-spaces)
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 47-47: trailing spaces
(trailing-spaces)
[error] 53-53: trailing spaces
(trailing-spaces)
[error] 62-62: trailing spaces
(trailing-spaces)
[error] 77-77: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (3)
.github/workflows/npm-publish.yml (3)
19-21:
Update deprecated workflow command.
The set-output command is deprecated. Use the new GITHUB_OUTPUT approach:
-run: echo "::set-output name=dir::$(yarn cache dir)"
+run: echo "dir=$(yarn cache dir)" >> $GITHUB_OUTPUT🧰 Tools
🪛 actionlint (1.7.4)
21-21: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
55-61:
Standardize NPM token usage.
The NPM token secret name should be consistent throughout the workflow. Choose either APPSMITH_NPM_PUBLISH_KEY or NPM_TOKEN and use it consistently:
-NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
+NPM_TOKEN: ${{ secrets.APPSMITH_NPM_PUBLISH_KEY }}67-77: 🛠️ Refactor suggestion
Add version validation before publishing.
Add a step to validate the package version before publishing to prevent accidental releases:
- name: Validate version
run: |
CURRENT_VERSION=$(node -p "require('./packages/design-system/ads/package.json').version")
NPM_VERSION=$(npm view @design-system/ads version 2>/dev/null || echo "0.0.0")
if [ "$(printf '%s\n' "$NPM_VERSION" "$CURRENT_VERSION" | sort -V | head -n1)" = "$CURRENT_VERSION" ]; then
echo "Error: New version ($CURRENT_VERSION) must be greater than published version ($NPM_VERSION)"
exit 1
fi🧰 Tools
🪛 yamllint (1.35.1)
[error] 77-77: no new line character at the end of file
(new-line-at-end-of-file)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
.github/workflows/npm-publish.yml (3)
14-17: Add working directory validationAdd a check to ensure the working directory exists before proceeding.
defaults: run: working-directory: app/client + - name: Validate working directory + run: | + if [ ! -d "app/client" ]; then + echo "Error: Working directory app/client does not exist" + exit 1 + fi
66-68: Add support for dry run modeConsider adding a dry run mode for testing the release process:
env: NPM_TOKEN: ${{ secrets.NPM_TOKEN }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + DRY_RUN: ${{ github.event_name == 'pull_request' }}🧰 Tools
🪛 yamllint (1.35.1)
[error] 68-68: no new line character at the end of file
(new-line-at-end-of-file)
11-13: Add error notificationsConsider adding error notifications (e.g., Slack) for build and publish failures:
release: name: Release runs-on: ubuntu-latest + if: ${{ always() }} + steps: + - name: Notify on failure + if: ${{ failure() }} + uses: slackapi/slack-github-action@v1.24.0 + with: + payload: | + { + "text": "❌ NPM publish workflow failed" + } + env: + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/npm-publish.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/npm-publish.yml
21-21: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
🪛 yamllint (1.35.1)
.github/workflows/npm-publish.yml
[error] 9-9: trailing spaces
(trailing-spaces)
[error] 22-22: trailing spaces
(trailing-spaces)
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 47-47: trailing spaces
(trailing-spaces)
[error] 53-53: trailing spaces
(trailing-spaces)
[error] 68-68: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (3)
.github/workflows/npm-publish.yml (3)
1-9: Add explicit permissions to the workflow
The workflow needs explicit permissions to function correctly.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 9-9: trailing spaces
(trailing-spaces)
18-22: Update deprecated workflow command
The set-output command is deprecated.
🧰 Tools
🪛 actionlint (1.7.4)
21-21: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
🪛 yamllint (1.35.1)
[error] 22-22: trailing spaces
(trailing-spaces)
54-57: Add version validation before publishing
Add a validation step to prevent accidental releases of invalid versions.
| # Create PR & publish to NPM | ||
| - name: Create release Pull Request or publish to NPM | ||
| id: changesets | ||
| uses: changesets/action@v1 | ||
| with: | ||
| publish: yarn release | ||
| commit: "ci(changesets): version packages" | ||
| title: "ci(changesets): version packages" | ||
| env: | ||
| NPM_TOKEN: ${{ secrets.NPM_TOKEN }} | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add retry mechanism for npm publish
NPM registry operations can be flaky. Consider adding retry mechanism:
- name: Create release Pull Request or publish to NPM
id: changesets
uses: changesets/action@v1
with:
- publish: yarn release
+ publish: npx --no-install retry-cli@0.7.0 -- yarn release
commit: "ci(changesets): version packages"
title: "ci(changesets): version packages"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Create PR & publish to NPM | |
| - name: Create release Pull Request or publish to NPM | |
| id: changesets | |
| uses: changesets/action@v1 | |
| with: | |
| publish: yarn release | |
| commit: "ci(changesets): version packages" | |
| title: "ci(changesets): version packages" | |
| env: | |
| NPM_TOKEN: ${{ secrets.NPM_TOKEN }} | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |
| # Create PR & publish to NPM | |
| - name: Create release Pull Request or publish to NPM | |
| id: changesets | |
| uses: changesets/action@v1 | |
| with: | |
| publish: npx --no-install retry-cli@0.7.0 -- yarn release | |
| commit: "ci(changesets): version packages" | |
| title: "ci(changesets): version packages" | |
| env: | |
| NPM_TOKEN: ${{ secrets.NPM_TOKEN }} | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
🧰 Tools
🪛 yamllint (1.35.1)
[error] 68-68: no new line character at the end of file
(new-line-at-end-of-file)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
app/client/package.json (1)
65-65: Consider moving @changesets/cli to devDependenciesThe @changesets/cli package is a development tool used for version management and changelog generation. It's not required at runtime, so it should be in devDependencies instead of dependencies.
Apply this change:
- "@changesets/cli": "^2.27.10",Add to devDependencies section:
+ "@changesets/cli": "^2.27.10",app/client/packages/design-system/ads/package.json (1)
14-16: Consider including additional package filesThe
filesfield should include other essential files like README, LICENSE, and type definitions."files": [ - "build" + "build", + "README.md", + "LICENSE", + "*.d.ts" ],app/client/packages/design-system/ads/src/Menu/Menu.types.ts (1)
Consider documenting this API change and removing the underscore prefix
The interface
_MenuItemPropsappears to be only used internally within the Menu component files. Making it public is indeed a breaking change that should be documented. Since it's becoming part of the public API:
- Remove the underscore prefix to follow TypeScript naming conventions for public interfaces
- Add an entry in CHANGELOG.md under a "Breaking Changes" section
- Consider adding more detailed JSDoc comments to document the interface properties
🔗 Analysis chain
Line range hint
25-35: Consider documenting this API changeMaking
_MenuItemPropspublic is a breaking change that should be documented in the changelog. The underscore prefix typically indicates internal/private usage, consider removing it if this interface is now part of the public API.
Let me check if this interface is used in other files or if there are similar patterns in the codebase.
Let me check the actual usage of this interface and the changelog content.
Let me check how this interface is used in the Menu component and look for any existing changelog entries.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for existing usage of _MenuItemProps to assess impact rg -l "_MenuItemProps" --type tsLength of output: 93
Script:
#!/bin/bash # Check for similar underscore-prefixed exported types ast-grep --pattern 'export interface _$_' # Check if this file is imported elsewhere rg -l "from ['\"].*Menu\.types['\"]" --type ts # Look for any changelog files fd "CHANGELOG" --type fLength of output: 410
Script:
#!/bin/bash # Check how _MenuItemProps is used in Menu.tsx ast-grep --pattern '$_: _MenuItemProps' # Check the changelog content cat app/client/packages/design-system/ads/CHANGELOG.mdLength of output: 111023
Script:
#!/bin/bash # Check how _MenuItemProps is used in Menu.tsx rg -A 5 "_MenuItemProps" app/client/packages/design-system/ads/src/Menu/Menu.tsx # Check recent changelog entries head -n 50 app/client/packages/design-system/ads/CHANGELOG.mdLength of output: 208
app/client/packages/design-system/ads/rollup.config.cjs (2)
22-23: Improve external dependencies configurationThe current external function is too broad. Consider explicitly listing external dependencies or using a more specific pattern matching.
- external: (id) => - !id.startsWith(".") && !id.startsWith("/") && !id.includes("src"), + external: [ + 'react', + 'react-dom', + '@radix-ui/react-dropdown-menu', + // Add other specific external dependencies + /^@appsmith\//, // All @appsmith scoped packages + ],
29-34: Consider adding PostCSS configuration fileMove PostCSS configuration to a separate
postcss.config.jsfile for better maintainability and reusability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
app/client/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
app/client/.changeset/README.md(1 hunks)app/client/.changeset/config.json(1 hunks)app/client/package.json(1 hunks)app/client/packages/design-system/ads-old/package.json(1 hunks)app/client/packages/design-system/ads/.npmignore(1 hunks)app/client/packages/design-system/ads/CHANGELOG.md(1 hunks)app/client/packages/design-system/ads/package.json(1 hunks)app/client/packages/design-system/ads/rollup.config.cjs(1 hunks)app/client/packages/design-system/ads/src/List/List.stories.tsx(1 hunks)app/client/packages/design-system/ads/src/Menu/Menu.types.ts(1 hunks)app/client/packages/design-system/headless/package.json(1 hunks)app/client/packages/design-system/theming/package.json(1 hunks)app/client/packages/design-system/widgets-old/package.json(1 hunks)app/client/packages/design-system/widgets/package.json(1 hunks)app/client/packages/icons/package.json(1 hunks)app/client/packages/storybook/package.json(1 hunks)app/client/packages/utils/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (12)
- app/client/packages/design-system/widgets/package.json
- app/client/packages/design-system/headless/package.json
- app/client/packages/design-system/widgets-old/package.json
- app/client/packages/design-system/ads-old/package.json
- app/client/packages/design-system/theming/package.json
- app/client/.changeset/README.md
- app/client/packages/design-system/ads/CHANGELOG.md
- app/client/packages/icons/package.json
- app/client/packages/utils/package.json
- app/client/.changeset/config.json
- app/client/packages/design-system/ads/.npmignore
- app/client/packages/storybook/package.json
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/packages/design-system/ads/rollup.config.cjs
[error] 1-1: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 1-2: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 2-3: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 3-4: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 4-5: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 5-6: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 6-7: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 7-8: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 8-9: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 9-10: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 11-47: Illegal use of an export declaration outside of a module
not allowed inside scripts
(parse)
🔇 Additional comments (5)
app/client/packages/design-system/ads/src/List/List.stories.tsx (1)
3-4: LGTM! Import paths updated for NPM package structure.
The relative imports align with the NPM package publishing requirements.
Let's verify the import pattern consistency across other story files:
✅ Verification successful
The previous command didn't find any @appsmith/ads imports, which actually supports that the imports have been updated consistently. However, let's verify this by checking the actual import patterns in all story files and specifically looking for relative imports to confirm the consistency of the changes.
Import paths are consistently using relative paths across all story files
The verification shows that all story files in the design system are consistently using relative imports (using "./", "../") instead of absolute paths. The changes in List.stories.tsx align with this pattern, making it suitable for NPM package publishing.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other story files follow the same import pattern
rg -g '*.stories.tsx' -A 1 'import.*from.*@appsmith/ads' app/client/packages/design-system/ads/src/
Length of output: 101
Script:
#!/bin/bash
# Let's check all story files in the design system for their import patterns
fd '.stories.tsx$' app/client/packages/design-system/ads/src/ --exec head -n 10 {}
# Also check specifically for relative imports in stories
rg -g '*.stories.tsx' 'import.*from "\.' app/client/packages/design-system/ads/src/
Length of output: 22076
app/client/packages/design-system/ads/package.json (3)
11-12: Verify rollup configuration and build output
The build script references a rollup config, but we need to ensure it's properly set up.
✅ Verification successful
Rollup configuration is properly set up
The rollup configuration is well-structured with:
- ESM output format with sourcemaps
- TypeScript support with declaration files
- Proper handling of assets (CSS, images, fonts)
- Minification and optimization enabled
- Correct peer dependencies handling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check rollup config existence and format
fd -t f rollup.config.cjs app/client/packages/design-system/ads/
Length of output: 122
Script:
#!/bin/bash
# Check rollup config content
cat app/client/packages/design-system/ads/rollup.config.cjs
Length of output: 1594
3-3: Verify version compatibility with npm publish workflow
The alpha version suggests this is a pre-release. Ensure the npm publish workflow is configured to handle pre-release versions correctly.
✅ Verification successful
Alpha version is appropriate for pre-release package
The version bump to 2.1.44-alpha.4 is correct for this package. The package is marked as non-private and the alpha tag indicates this is a pre-release version, which is appropriate for a design system package undergoing changes with the new rollup build configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the workflow handles alpha releases
rg -A 5 "npm.*publish" .github/workflows/
Length of output: 43
Script:
#!/bin/bash
# Let's check for npm publish related configurations in package.json files
rg -g "package.json" "publishConfig|\"private\":|\"public\":" -A 2
# Also check for any npm related workflow files
fd workflow .github/
Length of output: 3126
Script:
#!/bin/bash
# Let's check the specific package.json for the ads package
cat app/client/packages/design-system/ads/package.json
# Also check for any npm related workflow files in more detail
fd -t f . .github/workflows/ -x cat {}
Length of output: 67834
21-32: Review dependency versions for updates
Some rollup plugins are using older versions. Consider updating to the latest stable versions for security and features.
✅ Verification successful
Let me check the versions of other rollup-related dependencies to get a complete picture.
Dependencies are up to date
All rollup-related dependencies in package.json are using their latest stable versions. No updates are required at this time.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest versions of rollup plugins
for pkg in @rollup/plugin-commonjs @rollup/plugin-typescript @rollup/plugin-terser; do
echo "Checking $pkg..."
npm view $pkg version
done
Length of output: 646
Script:
#!/bin/bash
# Check latest versions of remaining rollup plugins
for pkg in @rollup/plugin-image @rollup/plugin-url @svgr/rollup rollup-plugin-copy rollup-plugin-peer-deps-external rollup-plugin-postcss rollup; do
echo "Checking $pkg..."
npm view $pkg version
done
Length of output: 1837
app/client/packages/design-system/ads/rollup.config.cjs (1)
42-44: Verify font files existence
Ensure the font files exist in the specified source directory and consider adding error handling for the copy operation.
✅ Verification successful
Font files are present and correctly located
The font files exist in the correct source directory app/client/packages/design-system/ads/src/__assets__/fonts/ with multiple font variants (regular, bold, light, medium, semi-bold) in both .woff and .woff2 formats. The copy operation in the rollup config is properly configured.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if font files exist
fd . "src/__assets__/fonts" -t f
Length of output: 140
Script:
#!/bin/bash
# Let's first check if the directory structure exists at a different level
fd "__assets__" -t d
# Also check for any font files in the repository
fd -e ttf -e woff -e woff2 -e eot -e otf
Length of output: 10475
| "description": "This is the package for the design system that powers the Appsmith platform", | ||
| "main": "src/index.ts", | ||
| "sideEffects": false, | ||
| "scripts": { | ||
| "generate:component": "plop component", | ||
| "lint": "yarn g:lint", | ||
| "prettier": "yarn g:prettier", | ||
| "test:unit": "yarn g:jest" | ||
| "test:unit": "yarn g:jest", | ||
| "build": "rm -rf build && rollup -c --bundleConfigAsCjs" |
There was a problem hiding this comment.
It seems that bundleConfigAsCjs flag contradicts what is specified in the config. Please check it out.
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
This PR has been closed because of inactivity. |
Description
This PR adds the NPM publish workflow for publishing design system package.
Fixes #
Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Warning
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12398167479
Commit: a9be8d7
Cypress dashboard.
Tags: @tag.Sanity
Spec:
It seems like no tests ran 😔. We are not able to recognize it, please check workflow here.
Wed, 18 Dec 2024 17:33:03 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Improvements