-
Notifications
You must be signed in to change notification settings - Fork 22
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
babel 7 configuration format #30
Comments
Would you mind running |
This line https://github.com/detrohutt/babel-plugin-inline-import-graphql-ast/blob/678284600731bc77b632766f969a6033d9304700/plugin/index.js#L54 will attempt to load the user's My recommendation would be to drop the usage of
should expand to
and then
can be
|
Wow, thanks for taking the time to detail what needs to be done @loganfsmyth! Do you think this is likely to be a breaking change (major version bump) for my plugin? I mean do you think anyone would have been relying on their config being loaded by my plugin in some way? I'm guessing no since I don't use anything from the config other than my own plugin's options? |
Nope! It's a brand new feature in Babel 7, so I can't imagine anyone would be relying on it, and your code already skips reading |
here are both npm's and yarn's outputs for
Note i have the following in my
|
@elmpp Thanks! It looks like you're using the newest one. I just checked the changelog and it doesn't look like it should have affected my plugin directly. Hopefully what Logan suggested will fix it for you. I'm working on making that change now and I'll ping you here when I've published the new version. @loganfsmyth I didn't see anything in the babel-handbook about pulling |
Nope! That's been there for ages. The first argument includes everything you get from importing babel-core itself, along with some extra stuff. That said, if you want to avoid it to keep things minimal you could also do
|
@loganfsmyth Amazing. I wish I'd have known about |
@elmpp I just published |
it works! Thank you very much (also to @loganfsmyth) |
@elmpp Awesome! Thanks for reporting back. :) |
@loganfsmyth I'm working on a new feature to reduce the size of the output code and I ended up with the code below which reduces output size by about 35% but it requires runtime processing and seems unnecessarily complex.. is there a way to take advantage of the function buildVariableAST(graphqlAST, importName) {
const templateOptions = { placeholderPattern: /IMPORT_NAME|AST_EXPRESSION/ }
const buildAst = template(`var IMPORT_NAME = Function(AST_EXPRESSION)()`, templateOptions)
const minifyOptions = { compress: false, mangle: true }
const { code } = minify(`var code=${JSON.stringify(graphqlAST)}`, minifyOptions)
const minifiedASTString = code.replace('var code=', '').replace(';', '')
return buildAst({
IMPORT_NAME: t.identifier(importName),
AST_EXPRESSION: t.stringLiteral('return (' + minifiedASTString + ')')
})
} |
@detrohutt The best you can do on the side of the transform itself would be
which will force-enable the |
@loganfsmyth Amazing! Thanks for being so available for my questions. It's great to have such a wellspring of knowledge on call when I get stuck or just feel like I'm probably doing something sub-par. And thanks for all your great work on Babel! |
@loganfsmyth That worked beautifully. Mind if I credit you in the CHANGELOG/Release notes/Twitter announcement? |
If you'd like, don't let me stop you :) |
Hi there,
I'm trying to bumble my way through to having a working babel7-based setup but there seems to be something wrong the moment i pull in "babel-plugin-inline-import-graphql-ast"
Here is my
babel.config.js
(this is now the babel7 method to specify a project-wide babel config, in lieu of.babelrc
):Example usage of the graphql inlining:
Error text:
The text was updated successfully, but these errors were encountered: