-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
feat(medusa): Preserve node flags in develop command #1860
feat(medusa): Preserve node flags in develop command #1860
Conversation
🦋 Changeset detectedLatest commit: 440a9a8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
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 |
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.
Good 🎉 few suggestions
141a13d
to
834a0c0
Compare
I think it's now possible to remove the |
891906a
to
834a0c0
Compare
@srindom do you think that this could get a review at some point. It would avoid a user to have to remove the node modules from the peers in the plugin as explain in the documentation and instead use the preserve flag. wdyt? |
@olivermrbl is attempting to deploy a commit to the medusajs Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
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.
LGTM - appreciate the contribution 🚢
@adrien2p okay to merge?
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 for the contribution and sorry for the delay ❤️
What
I have added the node
--preserve-symlinks
flag to the child process that gets created when running the Medusadevelop
command.Why
To help streamline the process of creating a new plugin - as I found this was a noticeable stumbling block for me. Otherwise, it is not possible to run a Medusa server with the
develop
command while testing a plugin locally.How
Converted the "cross-spawn" package usage to Node's standard lib "child_process" fork method. This provides the child process with sensible defaults (current directory, current environment, standard i/o...) and passes node flags from its parent, as well as adding the
--preserve-symlinks
as one of the default flags to Node.Testing
I tested this by building the
medusa
package and copying the newly builtdevelop.js
file to mynode_modules/@medusajs/medusa/dist/commands
directory in my Medusa project. Certainly not the best workflow, but I was finding that when I linked a local version of the medusa package to my Medusa project the symlinks for my plugin would still get resolved whether or not the--preserve-symlinks
flag was passed... probably a knowledge issue with dependencies that I couldn't get my head around.This aims to fix issue #1859.
Also, with this PR, perhaps
cross-spawn
is no longer needed as a dependency?