-
Notifications
You must be signed in to change notification settings - Fork 554
Export jinja files as TS module at build time #1296
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
Conversation
coyotte508
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can probably revert the tsup changes from the previous PR? (eg the tsup.config.ts file)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could still be committed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel that committing it would not bring much value but could be annoying in case of merge conflicts etc. (and adds a bigger diff in PRs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm yeah i'd rather ignore it
|
Thanks for the quick review. I've pushed 8854b03 to remove the previously added |
|
cc @xenova btw. FYI this PR is a follow-up of this discussion #1255 (comment). We now export all jinja files into JS strings exported at build time. If jinja.js ever implements a |
This PR should fix https://github.com/huggingface-internal/moon-landing/pull/13013. It partially removes the structure introduced in #1255.
Current problem is that inference snippets are generated in the front-end. Since front-end cannot access file-system and therefore the jinja files, we have to find a workaround. This PR adds a build step which exports all jinja files into a single TS module. I've updated the
package.jsonfile so that now the snippets code should be available in any environment (both node and browser).cc @coyotte508 who suggested such a solution.
Tested it in
@tasks-genand "it works"For the record, the exported file (not committed in this PR) looks like this:
# packages/inference/src/snippets/templates.exported.ts