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

Remove "source" field from emitted loc object #13

Open
evan-scott-zocdoc opened this issue May 2, 2018 · 4 comments
Open

Remove "source" field from emitted loc object #13

evan-scott-zocdoc opened this issue May 2, 2018 · 4 comments

Comments

@evan-scott-zocdoc
Copy link

Not sure if this is possible or needed by the babel plugin, but it would be great to remove the "source" part of the "loc" object. It seems to just be bloat since the query is preprocessed into other objects.

@evocateur
Copy link

Near as I can tell, the source property is used to create unique cache keys during fragment processing. I think it's safe to eagerly normalize the value in this plugin, as it's mostly noisy whitespace in my experience.

@randing89
Copy link
Contributor

randing89 commented Aug 18, 2019

The source is taking bytes and computation for just generating a cache key. This can be optimized in compile time, for example, pre-generate a shorter hash instead of saving the full source.

const unsafeLocBody = hash(normalize(loc.source.body.substring(loc.start, loc.end)));
const loc = {
  start: 0,
  end: unsafeLocBody.length,
  source: {
    body: unsafeLocBody
  }
}

The drawback is that it is then tied to the implementation of graphql-tag. But I would still like to have it as an option. Maybe the plugin can provide a post-process callback so the user can do additional transformation to the generated object definition.

@bnjmnt4n
Copy link

Given that the purpose of this babel plugin is to remove imports of graphql-tag, I would assume that the whole loc object of the AST can be removed unless there are any consumers of the AST's loc details, which seem unlikely - although I stand to be corrected.

@jacobp100
Copy link

I think if you're only using Apollo, it's safe to remove. Would you accept a PR that changes the options to have strip: false | "none" | "source-whitespace" | "loc" (with true meaning source-whitespace for compatibility) and updating the docs to detail these options?

Happy to change naming too if you feel strongly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants