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
Implement "Records and Tuples" transform #12145
Implement "Records and Tuples" transform #12145
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/41210/ |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit cd25b57:
|
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.
Thanks Rick! I planned to port the plugin to this repo but never actually found the time to do so. I really appreciate you doing this!
Two things for the Babel team:
-
I have no idea about how licenses work and what it compatible with what. We already have other exceptions in our codebase (you can check them in
.gitignore
), but they are all MIT. -
Currently this plugin transforms records and tuples syntax to a call to the builtin, and loads the polyfill for such builtin function.
This makes this plugin "self-contained", however we might prefer to do something similar toregenerator-runtime
(or to other transforms relying on builtins): just inject a reference to the global, and then let our users load the polyfill themselves*. This would make the plugin polyfill-agnostic (however, only one polyfill currently exists), but it requires user to explicitly specify the polyfill.In this case, the plugin could be simplified to something like this:
export default declare(({ template, types: t }) => { api.assertVersion(7); return { name: "@babel/plugin-proposal-record-and-tuple", inherits: syntaxRecordAndTuple, RecordExpression(path) { path.node.type = "ObjectExpression"; path.replaceWith(template.expression.ast`Record(${path.node})`); }, TupleExpression(path) { path.replaceWith( t.callExpression(t.identifier("Tuple"), path.node.elements) ); } }; });
* "Themselves" could be using a new polyfill provider in https://github.com/babel/babel-polyfills, or just core-js
/es-shims
when they implement Record
/Tuple
support.
I'd love to include this in a YouTube video I want to make this weekend. (Channel with 10K subscribers.) Would be good timing with C# 9 and its records just out. Oh well. |
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.
One minor comment, but otherwise this looks good.
apologies for the delay, addressed the comment and resolved the merge conflicts, should be good to go! |
Thanks! And don't worry for the delay, we haven't scheduled 7.13 yet anyway. |
7101e79
to
75caf98
Compare
thanks for the review @JLHwung , I think I've addressed your comments. |
@rickbutton Thanks! Can you re-run |
@JLHwung done |
dff302d
to
37d3924
Compare
This PR adds a transform for the Stage 2 Record and Tuple syntax. By default, the transform will look for
Record
andTuple
in the global, but will optionally look up the names in a module, if either theimportPolyfill
option is set totrue
, or a polyfill module name is specified viapolyfillModuleName
.We have an experimental polyfill available at https://github.com/bloomberg/record-tuple-polyfill, which I intend to publish as the
@bloomberg/record-tuple-polyfill
package soon.We originally open sourced this work under the
Apache-2.0
license, and have included said license in the plugin's subdirectory.