Skip to content
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

when installing from git it should run denali install #434

Open
luxzeitlos opened this issue Feb 18, 2018 · 4 comments
Open

when installing from git it should run denali install #434

luxzeitlos opened this issue Feb 18, 2018 · 4 comments

Comments

@luxzeitlos
Copy link
Contributor

When doing

yarn add https://github.com/denali-js/denali.git

I would expect denali build to be executed.

I think this should be possible by using prepare instead of prepublishOnly inside package.json.

@davewasmer
Copy link
Collaborator

A few questions:

  1. Why are you adding Denali via yarn add?
  2. Why would you expect a build to be executed after adding an npm package?
  3. Would you expect a build to occur after adding Denali specific packages, or any package?

@luxzeitlos
Copy link
Contributor Author

Thats my usual pattern if I encounter a bug in a dependency, fix it, and then want to use that fix while I wait for a PR to merge.

Maybe I was not clear what I meant. I dont expect an denali build of my app, but of denali itself, so I can use it.

I expect builds because thats how npm works. Things that are not in the bundle may get compiled. This is also true for native modules for example. So I expect in general that a yarn add giturl will give me the same then yarn add npm-module-name. For that later the build step has already run before publishing, so I expect it to happen for the first after the git clone.

Related

@davewasmer
Copy link
Collaborator

Oh the hilarity that is npm hook script names 🤦‍♂️

In this scenario, Denali should handle what you are trying to accomplish automatically - if it's not, perhaps thats a bug.

Basically, when Denali builds, it checks to see if any of the addons are:

  1. missing their compiled output,
  2. are currently under test (i.e. the app being built is this addons dummy)
  3. are symlinked, or
  4. returning true from isDevelopingAddon()

If any of those cases are true, it compiles the addon on-the-fly as well, and will usually try to write the compiled result back to the addon's source directory. The result is, for git dependencies, that the first time you run denali build it will compile the addon and write the result into your node_modules folder. Subsequent builds will reuse that build result.

If you are trying to fix an issue and want your addon code to recompile on changes or every time on a fresh build (rather than just on the first build and reusing the cached output), I suggest either symlinking or marking isDevelopingAddon().

I'd like to avoid using prepare because it runs on every npm i. That means that we'd kick off the CLI to build every addon, every time. That would be unacceptably disruptive - the CLI takes ~1s to complete even basic commands, and an average addon build might take ~3-4s, which means adding ~20-30s to every npm i for an average app. And most of that would be unnecessary rebuilds.

We could try to do some kind of cache check - i.e. see if there is compiled output, and if so, skip the build and use that. But that is coarse grained logic at best (how do we know when the cached build result is stale?).

If we continue optimizing down this path, the end result is that we end up replicating the logic above that is currently in the CLI, only we still at least get the startup penalty of starting the CLI a half-dozen times per npm install.

@luxzeitlos
Copy link
Contributor Author

perhaps thats a bug.

probably it is. I'm talking about denali itself here. So inside a denali app rm -rf node_modules; yarn add https://github.com/denali-js/denali.git;yarn;denali server; crashes.

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

No branches or pull requests

2 participants