eik-lib / issues

Issues and discussions that span all of Eik
https://eik.dev
1 stars 0 forks source link

RFC 1 - Align package, npm and map commands #1

Open trygve-lie opened 3 years ago

trygve-lie commented 3 years ago

Currently there are different commands in the client to publish an Eik package, npm package and an import map. All these are more or less similar on the server but we treat them a little bit differently by storing them under different namespaces on the server to avoid name clashes between privately used names of packages and names of public NPM packages.

The current publish commands are as follow:

Currently there is also no information in the eik.json defining which type the configured code belongs too among the package types. In other words; as a developer one have no way to know what publish command to run to publish a package based on just looking at a configuration. This can cause a good amount of confusion and wrongly published packages over time.

I would like to suggest that instead of operating with different publish commands we introduce a required type field in the eik.json which holds a value defining if the config is a Eik package, Npm package or Import Map.

Example:

{
  "name": "my-app",
  "type": "pkg",
  "version": "1.0.0",
  "server": "https://assets.myeikserver.com",
  "files": {
    "index.js": "./scripts.js",
    "index.css": "./styles.css"
  },
  "import-map": "https://assets.myeikserver.com/map/my-map/1.0.0"
}

Then the current eik package, eik npm and eik map commands is deprecated, or even removed, from the client in favour of a new eik publish command. The new eik publish command use the new type field to publish the package to the correct namespace.

We should consider if the type should be required or not. If not required I believe that publishing as a Eik package as the default is the most appropriate action.

digitalsadhu commented 3 years ago

I think this is a good suggestion. I suggest we make it optional and default type to pkg for easiest compatibility and alignment with the most common use case. I wonder if we should name the pkg type a full word: "package"?

gingermusketeer commented 3 years ago

This sounds like a good idea. Makes it a bit easier to know where the assets are going to end up.

I do like package more than pkg but I also like that pkg is the same as the url prefix so a bit torn there. Would make the url module logic simple using pkg.

Is type the best name? Not sure what it means other than the url namespace. Would namespace or publishNamespace be more accurate?

alexanbj commented 3 years ago

This is a good move.

If I'm not mistaken, publish works right now (at least for eik packages?) and I've already been confused as to what the difference is between publish and package.

digitalsadhu commented 3 years ago

publish is just an alias for package. Publish was preferred initially to line up with people’s knowledge of how npm works. But package lines up better with the package namespace. However, if we change to a single command for all 3 then publish probably makes more sense.

digitalsadhu commented 3 years ago

I can see the wisdom of using a less generic word than type. namespace seems an improvement. @trygve-lie ?

trygve-lie commented 3 years ago

I wonder if we should name the pkg type a full word: "package"?

I agree.

I can see the wisdom of using a less generic word than type.

I can see that too.

Though; on the server this is already established as type and used multiple places to hold this information. Ex; from the function parsing the different parts in the URL: https://github.com/eik-lib/service/blob/master/lib/utils.js#L39

We do also tend to refer to what type of package something is.

alexanbj commented 3 years ago

I think I prefer type. It implies we might handle them differently, because they are in fact completely different types!

Namespace feels more like the scope of the package, eg finn/dialog.

digitalsadhu commented 3 years ago

works for me

digitalsadhu commented 3 years ago

DECISION

A type field will be added to eik.json with discrete options package, map and npm. CLI will be changed to respect this value. It will of course also be possible to add this type field to package.json as well.

The work needed is as follows:

trygve-lie commented 3 years ago

:+1: