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

Update ReadMe with info about usage in new environments #887

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

Conversation

FossPrime
Copy link

From #786 (comment)
Requested by @jimmywarting and @Qix-

This removes difficult to use ES5 conventions and prepares us for v5. As someone who started using Node after ES6 with ESM as much as possible, the code and the readme was hard to read.

ps your issue template is 5 years old and not particularly relevant today.

@jimmywarting
Copy link

there is quite many more chains of require calls in the readme

@FossPrime
Copy link
Author

there is quite many more chains of require calls in the readme

do you have merge access?

@jimmywarting
Copy link

there is quite many more chains of require calls in the readme

do you have merge access?

no

@Qix-
Copy link
Member

Qix- commented Jun 4, 2023

ps your issue template is 5 years old and not particularly relevant today.

You're right, I'll remove it. Thanks for the reminder :) The issue template about trash comments is also no longer relevant now that Github removed its "how to Github" courses that referred to this repository for some reason, encouraging people to post nonsense comments and issues.


As for the changes, it's all good, but you're missing the http import now. If you add it I'll merge this, though fair warning it won't show up on npm's site, just the repository for now, as it requires a release to do so, which is more of a religious ritual than it is a simple process given the fallout whenever a release for debug is cut.

@FossPrime
Copy link
Author

FossPrime commented Jun 5, 2023

Sorted. Sorry, I made the original PR in a rush to check for a pulse when destructuring Debug was causing unexpected results. I was intentionally making it as small and easy to merge as possible, just to see if anyone was here.

I came back here after I saw this https://feathersjs.com/guides/migrating.html#debugging

Anyways I added a note or two about ESM. I tested it using esmoduleinterop on and off.
https://stackblitz.com/edit/pr-debug-pseudo-esm?file=tsconfig.node.json,vite.config.ts&terminal=dev

I could have used the far more minimal vite.new/vanilla-ts but old habits die hard.

@FossPrime FossPrime changed the title Remove bad practices from ReadMe Update ReadMe with usage in new environments Jun 5, 2023
@FossPrime FossPrime changed the title Update ReadMe with usage in new environments Update ReadMe with info about usage in new environments Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants