SignalK / specification

Signal K is a JSON-based format for storing and sharing marine data from different sources (e.g. nmea 0183, 2000, seatalk, etc)
Other
91 stars 68 forks source link

fix: make package usable from git #499

Closed sarfata closed 5 years ago

sarfata commented 6 years ago

Description of the problem

Right now it is very hard to work with SignalK/specification from a git repository because the package is not usable when it's installed from git.

For example:

"dependencies": {
  "@signalk/nmea0183-utilities": "^0.6.0",
  "@signalk/signalk-schema": "git://github.com/sarfata/specification#b3502bf",

Leads to:

 Module not found: Error: Can't resolve '@signalk/signalk-schema' in '/home/circleci/repo/node_modules/@signalk/n2k-signalk'

That is because most of the useful files, including the main file dist/index.js are generated, and installing from a git dependency does not generate the files.

You can test this for yourself:

How I fixed it

To fix this we need to:

Discussion

Biggest problem with this change is that everyone who depends on signalk-schema will now depend on babel as well. We are making the dependency much bigger. However, I do not see a way around this if we care about the package being usable from git.

tkurki commented 6 years ago

I do my cross repo work with separate clones and npm linl. Do yoy really need git dependencies? For CI you do.

sarfata commented 6 years ago

Do you really need git dependencies? For CI you do.

Yes exactly. My problem is that I need the package somewhere for my CI to build, test and deploy.

Another solution would be to produce a .tar.gz and store it somewhere until the PRs are merged but it's manual and not ideal either.

I guess it's really a philosophical question of whether a package should be usable via git. I have not found any really authoritative answer on that.

tkurki commented 5 years ago

I really don't want to include babel & friends as real dependencies.

Would this work for us? https://stackoverflow.com/questions/48287776/automatically-build-npm-module-on-install-from-github https://github.com/styczynski/dumb-package

sarfata commented 5 years ago

Closing this. I do not think we want to merge it. Agree that making babel a dependency is not worth it here.