Enable use outside of node.js environments#186
Enable use outside of node.js environments#186ianthetechie wants to merge 2 commits intopelias:masterfrom
Conversation
Joxit
left a comment
There was a problem hiding this comment.
Hi, thank you for using the project and your contribution.
I think we cannot merge this, it making the workflow less easy to maintain. We use txt to get a better view when the data is updated (see #132 for example, which was a attempt to update WOF data before the unification)
For each data sources (libpostal and WOF) we have 3 levels:
- Source data you can find in
resources/{libpostal,whosonfirst} - Overrided data by pelias you can find in
resources/pelias/{libpostal,whosonfirst}(see #15) - Overrided custom data by us and git ignored (that's why it's empty and you think it's not used) you can find in
resources/custom/{libpostal,whosonfirst}(see #27)
I think if you want the same behaviour, it should not be in the git repository but can be generated before publishing on npm? 🤔
|
Hi @ianthetechie, thanks for opening the PR, unfortunately I agree with Jones. Its not clear what benefit would be gained by this change, if you are able to demonstrate a significant improvement in performance or memory usage by running it in WASM then we might consider adopting some of those changes but running the code outside of nodejs isn't a goal of the project. |
|
I just wanted to add that while 'everything is a file' resource method is incompatible with bundlers, it allows users to take a stock docker image and 'bind mount' files inside the container, this is a convenient method of allowing users to customize/extend Pelias without having to build & maintain their own docker images. |
|
Understood; no worries. |
|
Have a look at https://github.com/isp-nexus/mailwoman, Teffen reached out to me about modernizing this codebase & converting to ESM modules, I said it would be better as a fork, it might be easier for you to convert to WASM. I'd actually love to have all the Pelias code be more modern (such as my newer modules like https://github.com/missinglink/s2js) but it just seems like a lot of busy work for not a lot of gain, instead I think we'll just slowly chip away at making it more modern with ES6 features and not worry about it being CJS. |
|
Yeah makes sense. While performance is a consideration, better interop and modern tooling was the main motivation for this. And a hard dependency on Node APIs was the thing preventing it. For example:
I'll probably write up a blog post on some of these ideas later 😂 it's still early days for a lot of the tooling in the WASM space. Anyways, for now it's easy to maintain a minimal fork to make my integrations easier. |
Here's the reason for this change 🚀
I'm experimenting with running parts of the stack in different environments (ex: WebAssembly components) where filesystem access via the node.js APIs is either not possible or undesirable.
If this isn't in line with Pelias' goals, that's probably fine and I'll just maintain a fork, but it seems like a reasonable change that'd be useful to others, so here's a PR with the changes :)
Here's what actually got changed 👏
fsandpathimports..txtfor now to get the key. I can clean up / change this. It's just in a pragmatically working with minimal changes sort of state for now.Here's how others can test the changes 👀
NOTE: I have not tested for any other second order effects like memory usage due to node
requirecaching. (And I'm not 100% sure what I'd look for here to be honest; open to any feedback as you guys are much more into the node ecosystem than I.)