Skip to content

Nt 2008: Publishing#120

Closed
David Nalchevanidze (nalchevanidze) wants to merge 41 commits into
mainfrom
NT-2008
Closed

Nt 2008: Publishing#120
David Nalchevanidze (nalchevanidze) wants to merge 41 commits into
mainfrom
NT-2008

Conversation

@nalchevanidze
Copy link
Copy Markdown
Contributor

No description provided.

@wiz-inc-38d59fb8d7
Copy link
Copy Markdown

wiz-inc-38d59fb8d7 Bot commented Jan 26, 2026

Wiz Scan Summary

Scanner Findings
Vulnerability Finding Vulnerabilities -
Data Finding Sensitive Data -
Secret Finding Secrets -
IaC Misconfiguration IaC Misconfigurations 2 Low 1 Info
SAST Finding SAST Findings -
Software Management Finding Software Management Findings -
Total 2 Low 1 Info

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

Comment thread .github/workflows/publish-packages.yml Fixed
Comment thread .github/workflows/publish-packages.yml Fixed
Comment thread .github/workflows/publish-packages.yml Fixed
Comment thread .github/workflows/publish-packages.yml Fixed
Comment thread .github/workflows/publish-packages.yml Fixed
Comment thread .github/workflows/publish-packages.yml Fixed
Comment thread .github/workflows/publish-packages.yml Fixed
Comment thread .github/workflows/publish-packages.yml Fixed
Comment thread .github/workflows/publish-packages.yml Fixed
registry-url: 'https://registry.npmjs.org'

- name: Setup pnpm
uses: pnpm/action-setup@v4

Check warning

Code scanning / CodeQL

Unpinned tag for a non-immutable Action in workflow Medium

Unpinned 3rd party Action 'Release Pipeline' step
Uses Step
uses 'pnpm/action-setup' with ref 'v4', not a pinned commit hash
Comment thread .github/workflows/publish-packages.yml Fixed
Comment thread .github/workflows/publish-packages.yml Fixed
Comment on lines +8 to +10
"paths": {
"logger": ["../../lib/logger/src/index.ts"]
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This shouldn't be necessary on web-vanilla because it doesn't use the packages in this way, as that's pre-built.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's not generate files to manage versions... we should keep that out of the codebase. If this is intended to only happen in the build, we also need to make sure it never gets checked in.

A better alternative would be to use an environment variable that can be resolved to a string at build-time. It would then be fully transient and will not have any concrete side-effects outside the build artifacts.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd imagine this library would also need the tsup.config.ts and also the changes to the tsconfig.* files.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please do not change the build scripts for Web or React Native. They are already bundled and should not use tsup.

"react-test-renderer": "18.3.1",
"rimraf": "catalog:",
"tslib": "catalog:",
"tsup": "catalog:",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is not necessary for this package.

Comment on lines +3 to +8
"compilerOptions": {
"paths": {
"logger": ["../../lib/logger/src/index.ts"],
"mocks": ["../../lib/mocks/src/index.ts"]
}
},
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this necessary to add to every tsconfig.json file, when these packages are already registered in tsconfig.base.json?

Comment thread tsconfig.base.json
Comment on lines +26 to +27
"logger": ["./lib/logger/src/index.ts"],
"mocks": ["./lib/mocks/src/index.ts"]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is it necessary to add these entries, if we're going to add them as devDependencies to the other projects, and the packages were building fine without these entries before? Is it necessary for tsup or something?

Comment on lines -37 to -43
"logger": "workspace:*",
"p-retry": "catalog:",
"zod": "catalog:"
},
"devDependencies": {
"@vitest/coverage-v8": "catalog:",
"mocks": "workspace:*",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's not remove either of these; just move logger to devDependencies as discussed.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm a bit concerned these config files have been created and the other config files have been removed. TS settings need to be set differently for each module style. Can you verify whether tsup handles this internally, and isn't merely generating similar code twice?

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.

3 participants