-
Notifications
You must be signed in to change notification settings - Fork 17
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
no_babel_transform option & change entrypoint iso overloads to return void #286
no_babel_transform option & change entrypoint iso overloads to return void #286
Conversation
None, | ||
format!( | ||
" | ||
export function iso<T>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes me think that we should not be using error|ignore|warning for this, but instead have a "no_babel_transform": bool
option or something. Thoughts?
Anyway, happy to land this as is to unblock Randall, but if you're up for making ☝️ change, we can skip a step
Makes sense, I can make this change |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me, thanks for identifying this and getting it done. Just a few nits
on_missing_babel_transform: create_optional_validation_level( | ||
options.on_missing_babel_transform, | ||
), | ||
no_babel_transform: options.no_babel_transform, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's rebase this on main and regenerate the schema as well
" console.warn('iso: Unexpected invocation at runtime. Either the Babel transform ' + | ||
'was not set up, or it failed to identify this call site. Make sure it ' + | ||
'is being used verbatim as `iso`.'); | ||
return (clientFieldResolver: any) => clientFieldResolver;" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we modify this error message too? E.g. add "If you cannot use the babel transform, set options.no_babel_transform
to true
in your Isograph config."
c30a130
to
ecd3fe2
Compare
…m' of github.com:PatrykWalach/isograph into skip-iso-entrypoint-overloads-on-missing-babel-transform
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM shipittt
No description provided.