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

Add .cjs extension to calls to force CommonJS mode in Node v12+ #7

Merged
merged 1 commit into from
Jan 29, 2021

Conversation

rdmurphy
Copy link
Contributor

@rdmurphy rdmurphy commented Jan 16, 2021

Fixes #6.

@Qix- You were correct! Adding .cjs to all calls and writes of _do_hook and _detect_package_hooks didn't bother Node v10 (which just runs them as CommonJS as always), and it successfully makes both Node v12 and Node v14 aware.

I was just testing it out by uninstalling and reinstalling then triggering the commit hook in the repo I discovered this in, so might benefit from another person poking on it.

@Qix-
Copy link
Collaborator

Qix- commented Jan 29, 2021

Great! - sorry for the delay, I'll publish this right now. Thanks again for all the work :)

@Qix- Qix- merged commit 8479611 into vercel:master Jan 29, 2021
Qix- added a commit that referenced this pull request Jan 29, 2021
@Qix-
Copy link
Collaborator

Qix- commented Jan 29, 2021

Nevermind, no I won't 😅 it appears my publish access has been dropped, cc @leo.

@leerob
Copy link
Member

leerob commented Apr 8, 2021

I can make sure this gets released! Thank you 😄

@Qix-
Copy link
Collaborator

Qix- commented Apr 8, 2021

@leerob That's not quite what I asked. I used to have permissions to manage this package as I was the original author and the person still trusted to release this. It's not common in the Node community to completely strip someone's authorship from code they wrote - generally frowned upon 🙃 It's especially not what I expected from ZEIT.

Can you be more specific or clear about what kind of role I play with this package these days? It'd be beneficial to Vercel, myself, and most importantly the users of this package to be clear, as the changes made on #8 make it seem like I'm no longer wanted around here.

If someone wants to take over maintainership of this package, that's fine, let's just make that clear to the existing maintainers 😉

@leerob
Copy link
Member

leerob commented Apr 8, 2021

Context: I've only been at Vercel for 8 months, so I was not here in the ZEIT days.

I'm happy to re-add you as a maintainer of the package. I'm only jumping in here to help with releasing, as it will need to be transferred over the Vercel NPM organization, and we're not looking to open up NPM publishing permission outside of our organization (for security reasons).

Sorry for the confusion here and for folks not getting back to you in a timely manner. Leo/G are never really on GitHub anymore as they're pretty busy 😄

@leerob leerob mentioned this pull request Apr 8, 2021
@advanced-media advanced-media mentioned this pull request Sep 6, 2021
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.

_detect_package_hooks throws error with "type": "module" in package.json
3 participants