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

Allow loader overwrite #195

Closed
Tracked by #171
StephanHoyer opened this issue Oct 4, 2022 · 6 comments
Closed
Tracked by #171

Allow loader overwrite #195

StephanHoyer opened this issue Oct 4, 2022 · 6 comments
Assignees

Comments

@StephanHoyer
Copy link
Contributor

Hi, first of all, great project. We really like it.

But we have a few problems integrating it. The dynamic loader gives us real troubles. We use the node-version also in the browser since we want better control over the hyphenation and pretty much only hyphenate strings and do the rendering with mithril.js.

Currently the the loader autodetects the environment. This is not really useful for us. A better option would be to have an optional argument loader which pretty much has the same signature as readFile. This would greatly improve usability. It also would allow to prefetch/embed certain lang files and treeshare the loader out of the package and rely on the user solution for loading

@StephanHoyer
Copy link
Contributor Author

If you like this idea, I would create a PR for this

@mnater
Copy link
Owner

mnater commented Oct 4, 2022

Hi

This sounds very interesting.
The current loader feels a bit hacky and not very idiomatic (I‘m not a node pro).

I look forward to your pull request.
This would be a nice feature for version 5, that I‘m currently working on…

Kind regards,
Mathias

PS: feel free to communicate in german.

@mnater mnater self-assigned this Oct 4, 2022
@mnater mnater added this to the next major release milestone Oct 4, 2022
@StephanHoyer
Copy link
Contributor Author

Ok, ich schaue wann ich das schaffe. Vermutlich aber noch diese Woche.

@StephanHoyer
Copy link
Contributor Author

Noch keine Zeit gefunden. :/

@mnater
Copy link
Owner

mnater commented Oct 10, 2022

Es eilt ja auch nicht.
Als ich schrieb, dass das in die Version 5 reinpasst, meinte ich vor allem, dass wir damit eine grosse Freiheit haben, wenn wir das API ändern wollen...

StephanHoyer added a commit to StephanHoyer/Hyphenopoly that referenced this issue Oct 13, 2022
StephanHoyer added a commit to StephanHoyer/Hyphenopoly that referenced this issue Oct 13, 2022
@StephanHoyer
Copy link
Contributor Author

StephanHoyer commented Oct 13, 2022

@mnater done!

it's a minor-change

StephanHoyer added a commit to StephanHoyer/Hyphenopoly that referenced this issue Oct 13, 2022
@mnater mnater closed this as completed in 635466d Oct 16, 2022
mnater added a commit that referenced this issue Oct 20, 2022
Problem:
`import fs from "node:fs"` is not always feasible.

Solution:
Leave the mechanism to load wasm files to the user of the script.
It is required now to configure a `loader` function.

Notes:
- hyphenopoly.deno.js is not longer needed
- Tests and docs are updated accordingly
Fixes #195, Fixes #196
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants