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

GitHub installs from npm #60

Closed
KnorpelSenf opened this issue Dec 17, 2021 · 10 comments
Closed

GitHub installs from npm #60

KnorpelSenf opened this issue Dec 17, 2021 · 10 comments

Comments

@KnorpelSenf
Copy link

I prefer to develop code for Deno and with Deno tooling, but more than 90 % of my users are consuming my library from npm. (I'm currently using deno2node.) I was looking into what a migration to dnt would mean. Since #57 I'm pretty sure the module would work perfectly on Node once I get the config right.

However, it is a priority that Node users aren't second-class citizens. I don't want to have a much worse npm package in terms of tooling for my users, just because I'm natively targeting Deno. Here are a number of things that I believe to be worse when using dnt with all its magic to generate configuration files.

Currently, apart from actual development, the npm package looks perfectly like it's native Node stuff. A lot of things would turn into magic from a Node point of view with a migration:

  1. No package.json in repo, so it doesn't look like a repo that contains code which would run on Node.
  2. No GitHub installs because of 1.
  3. You must know the concepts of Deno to understand that there can be TypeScript but no tsconfig.json file
  4. No npm scripts for running formatting, linting, building, etc. My current scripts are using deno fmt --config deno.json, so people would still need to have the CLI installed, but npm scripts often serve as landmarks to tell people how the repo works. If I'd use dnt, they'd need to know what Deno is, how to invoke the CLI of Deno with all the commands, and need to know the --config option.

Basically, dnt may make the build output look like a Node module, but from a development point of view, people would have to have a fair bit of knowledge about Deno.

For example, when people report a bug or request a feature, we'll implement it on a feature branch. We can now tell them to install the feature branch into their node_modules folder to try out if that's what they expected. They just need to run npm install grammyjs/grammY#feature-branch and they're good to go. Once they confirm, we can merge and everything's well tested. It would not be feasible to have them

  • install Deno
  • clone the repo
  • run the build script
  • npm link the build output into their other project

just to try out a small fix.

Do you have any ideas how to tackle these problems?

@wojpawlik
Copy link

Could this be worked around by creating own top-level package.json?

@KnorpelSenf
Copy link
Author

Only if there was dnt on npm which would perform the compilation locally

@dsherret
Copy link
Member

dsherret commented Jan 10, 2022

No npm scripts for running formatting, linting, building, etc. My current scripts are using deno fmt --config deno.json, so people would still need to have the CLI installed, but npm scripts often serve as landmarks to tell people how the repo works. If I'd use dnt, they'd need to know what Deno is, how to invoke the CLI of Deno with all the commands, and need to know the --config option.

Recommend following the config tasks proposal for Deno: denoland/deno#12764

when people report a bug or request a feature, we'll implement it on a feature branch. We can now tell them to install the feature branch into their node_modules folder to try out if that's what they expected. They just need to run npm install grammyjs/grammY#feature-branch and they're good to go. Once they confirm, we can merge and everything's well tested. It would not be feasible to have them

  • install Deno
  • clone the repo
  • run the build script
  • npm link the build output into their other project

I think this is out of scope for dnt. If you want to let users try something out this way, you could create an empty branch in the git repo then put the output of dnt in there with the feature for them to try. Then users could run the command like you mentioned.

Alternatively, a separate tool could be created for node that downloads deno locally if it doesn't exist and then runs a build script that may include dnt. There could be a folder in the repo that people could point their npm clients at that contains a package.json that does all of this in a postinstall script. Then users could install doing something like:

# I forget what the proper way of writing this command is, but anyway
npm install https://github.com/username/repo/tree/branchName/folderContainingPackageJson

Edit: It's easier to just publish the package as <package_name>@next

@KnorpelSenf
Copy link
Author

KnorpelSenf commented Jan 10, 2022

I think this is out of scope for dnt. If you want to let users try something out this way, you could create an empty branch in the git repo then put the output of dnt in there with the feature for them to try. Then users could run the command like you mentioned.

This is missing the point. The problem is different.

Situation before targeting Deno and backporting via dnt: users can directly install the current code base into their project.
Situation after: they can't. They need a lot of knowledge and use a number of different tools for this.

The question is, how could dnt try to not make the situation worse, i.e. "how to install a particular project with a single command into my Node project". It does not matter if the answer is using a particular feature of the npm CLI or not.

Either way, polluting the git with build output is not a solution that should be the recommended approach. Better than creating a branch and pushing the JS files would be to always commit the package.json file. That way, this issue could be closed immediately, as postinstall scripts can perform the setup and compilation etc. deno-bin could be a dev dependency and dnt will simply run normally.

The longer I think about it, the less useful is the property of dnt to generate package.json files. I understand that coming from Deno, one wants to have a project without config file ("it just works") but the unfortunate reality is that Node projects do need config. Trying to somehow automatically generate things that dnt cannot know about is unlikely to yield good results. (The API of dnt even admits that, since most package.json options in turn need to be specified inline in the build script again.) Could you outline why dnt even tries to do this?

I fail to see how anyone who would build a serious project for Node.js would not want to be in control of their package.json file. The idea of just running dnt and being done may be cute for hobby projects, but in my experience, most larger projects with 5+ collaborators care a lot about their config file and iterate on it for some time to get it right for all things, such as module resolution; and TS vs. JS; and entrypoints; and prepare/postinstall scripts; and all the rest in there that can break. With dnt, I only have indirect access to the file, as I always have to inspect the build output (!) and then make sure dnt did the right thing.

This is confirmed by deno2node. It does leave the configuration to the user, and it simply does not have any of these problems. (In fact, people have been complaining that their VSCode does not work with my project, because it looks so compatible with Node, that they did not spot the difference. At the same time, we actually have a true Deno-first project. That's a level of Node compatibility that I don't see dnt to enable anytime soon, at least not until it changes its strategy in this respect.)

Alternatively, a separate tool could be created for node that downloads deno locally if it doesn't exist and then runs a build script that may include dnt. There could be a folder in the repo that people could point their npm clients at that contains a package.json that does all of this in a postinstall script. Then users could install doing something like:

# I forget what the proper way of writing this command is, but anyway
npm install https://github.com/username/repo/tree/branchName/folderContainingPackageJson

This is a better solution than manually pushing build output to GitHub whenever we want people to try something out, but again, this tool does not need to exist if you were just changing dnt to read in a package.json file, instead of generating it.

@dsherret
Copy link
Member

dsherret commented Jan 11, 2022

The point of dnt is to take a deno module and distribute it on npm. There is no need for a package.json in a deno-first module, which is why it's a build artifact.

This definitely seems out of scope for dnt because dnt is just a build function—it's not a toolchain and doesn't prescribe how your repo is setup. If you really want a package.json then you can have one and then supply its json data to dnt's build options. Then, with a package.json file you can develop some postinstall scripts that does everything you want for npm users to install it.

But again, even without having a package.json in the repo, there are many possible solutions to this problem. For example, it would be trivial to update a GitHub action workflow on the main branch to build using dnt (which it should already be doing for testing purposes) and then push that output to a bare branch (ex. main_npm). Documentation could then be updated to explain to users that they can install the current main branch using npm by installing the main_npm branch. This doesn't seem like "polluting" git to me to have one extra branch or to create a workflow that creates these branches on demand.

I fail to see how anyone who would build a serious project for Node.js would not want to be in control of their package.json file. The idea of just running dnt and being done may be cute for hobby projects, but in my experience, most larger projects with 5+ collaborators care a lot about their config file and iterate on it for some time to get it right for all things, such as module resolution; and TS vs. JS; and entrypoints; and prepare/postinstall scripts; and all the rest in there that can break.

All of this is completely configurable in dnt and you are in full control of the output of the package.json file. Shimming is completely configurable now and you can provide whatever contents you want to the package object. Additionally, you are free to modify the output in any way you wish.

With dnt, I only have indirect access to the file, as I always have to inspect the build output (!) and then make sure dnt did the right thing.

You should be writing many Deno.test cases. If type checking and the deno tests pass, then you should be able to have confidence in the output. That said, because dnt is still early days I would recommend inspecting the build output.

This is confirmed by deno2node. It does leave the configuration to the user, and it simply does not have any of these problems. (In fact, people have been complaining that their VSCode does not work with my project, because it looks so compatible with Node, that they did not spot the difference. At the same time, we actually have a true Deno-first project. That's a level of Node compatibility that I don't see dnt to enable anytime soon, at least not until it changes its strategy in this respect.)

This doesn't sound great to me. In my opinion, it's more ideal to have a deno-first project that looks like a deno-first project and not confuse people by looking like a node project.

@dsherret
Copy link
Member

dsherret commented Jan 13, 2022

I'm going to close this one as I believe it's out of scope. To summarize:

  1. Deno and not dnt is the desired toolchain. Follow Proposal: add tasks to the config file deno#12764 for updates on the config task proposal.
  2. Recommend either outputting the npm code to a branch or folder for git based installs. Edit: Use <package_name>@next feature of npm.
  3. dnt does not dictate whether you have a package.json file in your repo or not.
  4. Someone could create a script that downloads deno in node_modules similar to how esbuild works. That would make Deno an npm dev dependency and building the dnt output using Deno could be possible for users without needing to install Deno. This is out of scope for dnt though.

@KnorpelSenf
Copy link
Author

Understood.

There are 20+ hybrid repos in our org that target both Deno and Node, and they all have a slightly different setup. However, the suggestions do not satisfy as solutions to any of these projects. Hence, I conclude that dnt is not the right tool for the job, so we are going to stick with deno2node.

Thank you for your elaborations!

@wojpawlik
Copy link

What would it take to publish @deno/dnt on npm?

@wojpawlik
Copy link

Alternatively, could dnt have an option to generate package.json outside npm/?

@wojpawlik
Copy link

Consider mentioning deno2node in dnt's Readme

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

3 participants