indic-transliteration / sanscript.js

Transliteration package for Indian scripts
MIT License
98 stars 39 forks source link

Installing from git dependency is broken #54

Closed grosmar closed 2 years ago

grosmar commented 2 years ago

When installing from git (i.e. npm i indic-transliteration/sanscript.js), then it's broken, as the prepare script is running transpile script, which will need the scheme files stored in the submodule, but npm doesn't handle submodules anymore.

Best proposal would be to reference schemes as npm package, and not a submodule.

What do you think?

vvasuki commented 2 years ago

https://github.com/indic-transliteration/common_maps is not specific to js or npm. So, I don't like the idea of adding npm packaging files there. What are the alternatives?

grosmar commented 2 years ago

I understand your concern, however beside the fact package.json would be a bit of "noise" there, it doesn't have any other effect on the repo itself.

Other options:

So I still would propose my first solution and introduce a package.json in the common_maps, unless you are extremely against it.

vvasuki commented 2 years ago

Is it just a matter of adding a single file (package.json)?

How would local development work? (Consider a workflow where you alter a schema definition, add and run some tests - what steps would be involved?)

grosmar commented 2 years ago

Yes, it would be just an extra package.json file

Check out the implementation proposal: https://github.com/grosmar/sanscript.js/blob/schema-dependecy/package.json#L117 https://github.com/grosmar/common_maps/tree/package-json

As you can see, there is a dependency to the github repository, submodule is removed, build file modified to point into the node_modules/common_maps

Regarding the local workflow, you can use npm link check out here Even simpler than modifying all the time the git submodule commit hash.

You simply work in your local directory of the dependency package and it's effect is immediate (of course only locally).

vvasuki commented 2 years ago

I suppose that since npm build and publish will continue to work fine? If so, please go ahead and send a PR. Also update README.md to remove "Adding new schemes" section and add a pointer to npm link.

grosmar commented 2 years ago

Sure, everything should work prefectly. Build already modified for this, I guess the publish is using the build script, so it should be fine, but that part should be checked by you after I send the PR, as I have no rights to publish to your npm repo.

I will finish the implementation and send the PRs

grosmar commented 2 years ago

Created 2 PR-s.

  1. First need to merge common_maps PR,
  2. Release common_maps to npm under the name @sanskrit-coders/common_maps
  3. Merge the second PR I created for sanscript.js
vvasuki commented 2 years ago

Done and published to https://www.npmjs.com/org/indic-transliteration (old package location deprecated.)