Skip to content

Comments

bump node#653

Merged
tharvik merged 2 commits intodevelopfrom
633-bump-node-tharvik
Mar 25, 2024
Merged

bump node#653
tharvik merged 2 commits intodevelopfrom
633-bump-node-tharvik

Conversation

@tharvik
Copy link
Collaborator

@tharvik tharvik commented Mar 21, 2024

currently, we're running with node 16, which is not LTS anymore. some packages requires newer nodejs, wrtc is dead (and not compatible with node 16 anyway), so upgrading is needed.

@tharvik tharvik self-assigned this Mar 21, 2024
@tharvik tharvik force-pushed the 633-bump-node-tharvik branch 2 times, most recently from 56dc8d2 to 59b2eec Compare March 21, 2024 08:37
@tharvik tharvik force-pushed the 633-bump-node-tharvik branch from 79e4a3d to 737339a Compare March 21, 2024 10:11
@tharvik
Copy link
Collaborator Author

tharvik commented Mar 21, 2024

node-datachannel requires esm modules, moving it #652.
hopefully, bumping to nodejs 20 won't segfault too much

@tharvik tharvik marked this pull request as ready for review March 21, 2024 10:19
@tharvik tharvik requested a review from JulienVig March 21, 2024 10:20
Copy link
Collaborator

@JulienVig JulienVig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

I've quickly tried to import and use Transformers.js but I get an import error.
It's the same as in this stackoverflow post, which is answered by Transformers.js's author
Following the answer I can circumvent the issue like that:

const TransformersApi = Function('return import("@xenova/transformers")')()
const { AutoTokenizer } = await TransformersApi
const tokenizer = await AutoTokenizer.from_pretrained('Xenova/bert-base-uncased')
const { tokens } = await tokenizer('I love transformers!')

The Function import triggers a linting error and honestly looks quite hacky
What do you think? I'm bringing this up here in case we need to change things before merging

@tharvik
Copy link
Collaborator Author

tharvik commented Mar 21, 2024

I've quickly tried to import and use Transformers.js but I get an import error.

haa, welcome to the ESM (ECMAScript Module) world! this error means that node is trying to read an ESM module as a CommonJS one (old node-specific module system). to be able to load it, we need ESM support (which requires to change typescript target & module to be at least ES2015). and that's what I'm trying to fix with #652.

The Function import triggers a linting error and honestly looks quite hacky What do you think? I'm bringing this up here in case we need to change things before merging

this is indeed quite hacky, does it work at all?
anyway, if it helps you progress, keep it for now, and (as I'll probably forget), remember me to remove it when reviewing #652

@JulienVig JulienVig self-requested a review March 25, 2024 09:46
Copy link
Collaborator

@JulienVig JulienVig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good! Comments are resolved by #652

@tharvik tharvik merged commit 6b2fea1 into develop Mar 25, 2024
@tharvik tharvik deleted the 633-bump-node-tharvik branch March 25, 2024 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants