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.

Sign up for GitHub

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

Jump to bottom

js-eval: remove h>, simplify stuff #55

Merged
merged 4 commits into from Dec 29, 2020
Merged

js-eval: remove h>, simplify stuff #55

merged 4 commits into from Dec 29, 2020

Conversation

caub
Copy link
Contributor

@caub caub commented Dec 28, 2020 •
edited

  • Remove h> command, --harmony flag doesn't even exist anymore in node 14+
  • We need to run code using node15, both in the main bot process and in docker if possible? because using require('timers/promise')
  • I'll update babel plugins also in a 2nd commit

@caub caub force-pushed the rem-h branch 2 times, most recently from 4e07912 to fa97a64 Compare December 28, 2020 22:13
@brigand
Copy link
Owner

brigand commented Dec 28, 2020

Let's keep the bot compatible with 12.x for now. I should upgrade the server to 14.x at some point, but not going to do odd majors.

@caub
Copy link
Contributor Author

caub commented Dec 28, 2020

I'll update the code in jsEvalPlugin so, ok

caub
caub commented Dec 28, 2020
View reviewed changes
src/plugins/js-eval/jsEvalPlugin.js
@@ -33,23 +29,20 @@ module.exports = async function jsEvalPlugin({

const hasMaybeTLA = /\bawait\b/.test(code);

if (mode === 'b' && !hasMaybeTLA) {
code = (await babel.transformAsync(code, { plugins: transformPlugins })).code;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

done a fix, because that code above was not using parserOpts: {plugins: parserPlugins} before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should allow us to use records and tuples, or other babel syntax things, but I'm not totally sure

@caub
Copy link
Contributor Author

caub commented Dec 28, 2020 •
edited

Ok done, the docker image can use node15 right? (still using timers/promises there, but could update without if needed)

@caub caub force-pushed the rem-h branch 2 times, most recently from 0afb100 to 6c3da1a Compare December 29, 2020 10:13
@caub
Copy link
Contributor Author

caub commented Dec 29, 2020 •
edited

Hmm for record&tuple, only syntax support was added babel/babel#10865, it's not transpiled to anything yet https://repl.it/@caub/babel-record-and-tuple#index.js, so we can't evaluate it, maybe engine262 has it though (edit: not yet too)

@caub
Copy link
Contributor Author

caub commented Dec 29, 2020 •
edited

Guys, Record and Tuple is not ready yet currently, it'll be soon babel/babel#12145

I'll make a different PR for that when the time will come

@brigand brigand merged commit 70d8a9c into brigand:master Dec 29, 2020
@brigand
Copy link
Owner

brigand commented Dec 29, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brigand brigand brigand approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@caub @brigand @ljharb