Improves deployment logic for handling alternate dependency protocols.#1559
Improves deployment logic for handling alternate dependency protocols.#1559jeremyzahner wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 774f8da 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 |
jeremyzahner
force-pushed
the
feat/deployment-dependency-resolution-improvements
branch
2 times, most recently
from
dd992fe to
8945017
Compare
jeremyzahner
force-pushed
the
feat/deployment-dependency-resolution-improvements
branch
from
8945017 to
b5f47a0
Compare
SamuelAlev
requested review from
mike85 and
ragi96
and removed request for
Kenny806,
SedukhinaPolina and
silviojaeger
|
@jeremyzahner please sign your commits :) |
@ragi96 Sorry, did press the Rebase Button on GH. Will do asap. |
| buildFilesToIgnore, | ||
| ); | ||
|
|
||
| Logger.info('Resolved dependencies:'); |
There was a problem hiding this comment.
@jeremyzahner I’d lean towards removing these logs. They add quite a bit of noise to the console without bringing much value for most developers, since they don’t really need insight into these internals.
If there’s still a need for this kind of visibility, we could always introduce a --verbose (or debug) mode later where such logs are more intentional and helpful.
Would that approach work for you?
There was a problem hiding this comment.
Will remove them for now. Maybe set up a separate PR for this later on.
| console.error(pc.red(`[${getCurrentTime()}] ${messages.join(' ')}`)); | ||
| } | ||
|
|
||
| static warn(...messages: string[]): void { |
There was a problem hiding this comment.
This one should only be logged behind a --verbose flag
Feel free to create a new PR to add this mode 👍
There was a problem hiding this comment.
@ragi96 Looking at how i used it right now (for example warning about potentially sensitive files being leaked) I'm not sure I would see it as being a "verbose" case. I would treat this as a "warning" much rather than as a "debug" message of sorts which should absolutely be placed behind a flag like this (like the verbose logs i removed about the source file list ie.). Do you want me to remove this entirely for now?
Signed-off-by: Jeremy Zahner <zahner@joshmartin.ch>
jeremyzahner
force-pushed
the
feat/deployment-dependency-resolution-improvements
branch
from
b5f47a0 to
ba8fefa
Compare
3 participants
Hey guys!
Here my follow up from the last PR. When starting to use the new SDK revision, we noticed we we're still having runtime issues with the blocks after deployment (using the
catalogprotocol in our package.json). My last fix solved the issue when using the dev server, but not for deployments. This should solve that aspect as well.For now it's a draft since i wanted to speak to @floriangaechter before marking it ready.Edit 1 -
One question: For better observability I've added logging for resolved deps, source files and dist files when deploying. I think its generally useful for users deploying to have more transparency. However i see that this is somewhat opinionated, so let me know if I should remove it.-> Removed the verbose logs.Edit 2 - Also as a remark: As per other setups even today, this does not fix the issue of not all source files (ie. shared monorepo libs) being deployed to enable easier reviews. This is simply out of scope for this MR. I understand that this is an ongoing discussion. These changes however enable working deployments out of workspace/catalog monorepos without user needing to implement workarounds for using default pnpm features.
"@frontify/frontify-cli": patch
fix(cli): resolve workspace protocol specifiers during deployment
The deploy command now resolves
catalog:,workspace:,link:,file:, and other protocol specifiers to their actual installed versions before uploading to the Frontify Marketplace API. Previously, these raw specifiers were sent as-is, causing deployment failures in pnpm workspace setups.Key changes:
node_moduleslookup duringcollectFiles. Unresolvable protocol specifiers are omitted from the payload with a warning.package.jsonincluded insource_filesis sanitized with resolved versions before upload..env,.npmrc,.netrc) are detected in the source upload.