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

upgrade eslint and fix tsconfig.lib #167

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

Jomik
Copy link
Contributor

@Jomik Jomik commented Oct 11, 2022

No description provided.

@Jomik Jomik changed the title upgrade and fix tsconfig.lib upgrade eslint and fix tsconfig.lib Oct 11, 2022
@pyrodogg
Copy link
Member

I think the target should stay at ES2018. Node 10 supports ES2018 in all of the major ways.

I've done more research on this than I'm happy about. Node 10 should be dead, it stopped receiving security patches on 30 Apr 2021. So researching it is getting more frustrating as time goes on (ex. I cannot run tests on replit. because drumroll it's a (potentially) insecure package).

Node.green lists ES2018 as 92% supported for Node 10 but this I think is a very minor technicality. Supporting ES2018 allows for the following features to be used without transpiling, object spread and rest syntax, async generators, Promise.finally.

The Proxy ownKeys note is a rather minor footnote compat-table/compat-table#1543 Perhaps critical for
node/browsers to get right but for devs it's more of a "don't do this, will cause obvious bugs" sort of indicator.


For engine, it would be better to remove it altogether. I really don't want to advocate for more use of Node 10. Players should be free to develop with whatever they want. At most, it was most an indication of what your "output" is expected to run in which would be Screeps MMO running on Node 10.

I haven't had time yet to test the rest of the changes but I suspect the dependency updates are fine, will check on the next pass.

@Jomik
Copy link
Contributor Author

Jomik commented Oct 26, 2022

Object spread and rest syntax is a good point! That at least makes the bundled code smaller in size, so arguably better.

Promises and async generators do not really matter for screeps though.


Completely agree on removing the engine field.

@Jeroendevr
Copy link

Can I bump this request to make this PR into the master branch?

@Jomik
Copy link
Contributor Author

Jomik commented Dec 9, 2022

I think the README may need to have an update, to remove the reference to what node version is required.

@Jeroendevr
Copy link

Seems a small thing to to, I would like to contribute to. But can I do that within this PR, is that possible? Or should I create a separate PR?
Seems like a small fix to just merge this PR and then adjust the readme or do I overlook something?

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.

3 participants