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

You have a global leak: __core-js_shared__ #364

Closed
geek opened this issue May 2, 2018 · 5 comments
Closed

You have a global leak: __core-js_shared__ #364

geek opened this issue May 2, 2018 · 5 comments

Comments

@geek
Copy link

geek commented May 2, 2018

No description provided.

@searls
Copy link
Member

searls commented May 2, 2018

I presume this is because we pull in common-tags, which pulls in babel-runtime, which pulls in core-js, which probably sets that global.

I would rather common-tags didn't pull in babel-runtime, but I don't think this is hurting anything.

@searls searls closed this as completed May 2, 2018
@geek
Copy link
Author

geek commented May 2, 2018

@searls thanks for the response. This is a regression from 3.7.0, if a dependency changed thats causing this issue are you open to pinning to the version of the dependency that doesn't leak into the globals?

@searls
Copy link
Member

searls commented May 2, 2018

common-tags is a new dependency as of 3.8.0, one I've been intending to add for a while, because multiline messages are all over this codebase and I'm trying to make it easier to add more of them by stripping indents.

One question and three options.

Question: is the existence of this transitive dep dropping something on global actually breaking anything?

Options:

  • Reimplement stripIndent inside tdjs here to remove the dep
  • Send a PR to common-tags to remove the production dep on babel-runtime resolve https://github.com/declandewet/common-tags/issues/108
  • Send a PR to core-js to remove the global, but I suspect based on the fact only one issue references the global at all, it's probably viewed as a feature not a bug

Seeing as core-js is installed 7M times per week and depended on by 5700+ open source packages, I think that's probably the place to raise the issue if the root cause worry is that a variable is being set to global and should not be.

Like I said earlier, I'd like to be rid of the dep as well, and think common-tags would be a stronger tool without needing babel-runtime. I'd also be happy to merge in or publish a standalone indent-stripping heredoc template tag function.

@searls
Copy link
Member

searls commented May 2, 2018

As a point of order, I appreciate you opening the issue to express your concern, but titles that single people out (e.g. "You have…") and issues with empty descriptions that don't explain the nature of the problem caused are often perceived as flippant or rude. Receiving them makes me less eager to help folks or empathize with the issues they're raising, especially if they don't impact me personally.

In the future, I'd really appreciate it if you put more detail about the impact of the issue is having on you and any suggestions you might have in issue description. Even if it feels obvious or duplicative, it's less likely to come across as entitled and less likely to make the maintainer feel like your order-taker.

searls added a commit that referenced this issue May 5, 2018
@searls
Copy link
Member

searls commented May 5, 2018

Hey @geek, I decided to create a standalone stripIndent implementation for heredocs called theredoc. It doesn't have any dependencies, and replaces common-tags as of testdouble@3.8.1, which just landed.

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

No branches or pull requests

2 participants