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

babel-plugin-async-to-generator should add #__PURE__ comment #6572

Closed
winstonewert opened this issue Oct 27, 2017 · 7 comments · Fixed by #6803
Closed

babel-plugin-async-to-generator should add #__PURE__ comment #6572

winstonewert opened this issue Oct 27, 2017 · 7 comments · Fixed by #6803
Labels
area: perf claimed good first issue outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@winstonewert
Copy link

Request: add pure comments to async function helpers to aid in dead code elimination.

Input Code

https://babeljs.io/repl/#?babili=false&browsers=&build=&builtIns=false&code_lz=IYZwngdgxgBAZgV2gFwJYHsLwKbKgCwAVgBzbAEWGWAAoAHU7AShgG8AoGGAJ1wW6ysAvuyFA&debug=false&circleciRepo=&evaluate=false&lineWrap=true&presets=es2015%2Creact%2Cstage-2&targets=&version=6.26.0

async function fetchPageData(page) {
  return {}
}

Expected Behavior

Generated code should be able to be removed as dead by uglifyjs/similiar if the async function is not called.

Current Behavior

var fetchPageData = function () {
  var _ref = _asyncToGenerator( /*#__PURE__*/regeneratorRuntime.mark(function _callee(page) {
    return regeneratorRuntime.wrap(function _callee$(_context) {
      while (1) {
        switch (_context.prev = _context.next) {
          case 0:
            return _context.abrupt("return", {});

          case 1:
          case "end":
            return _context.stop();
        }
      }
    }, _callee, this);
  }));

  return function fetchPageData(_x) {
    return _ref.apply(this, arguments);
  };
}();

Note that while regeneratorRuntime.mark is marked as #PURE, the call to _asyncToGenerator and the IIFE aren't.

Possible Solution

Generated code should start with:
var fetchPageData = /#__PURE/ function () {

Context

I have a react app which runs on both client and server. However, I have various async functions that only run on the server. I want to have them removed during the minification process.

@babel-bot
Copy link
Collaborator

Hey @winstonewert! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@hakz-00
Copy link

hakz-00 commented Oct 29, 2017

Hello, It seems no one has offered to provide a solution. Would you mind if I had a go at it? It is my first time contributing to open software, but I think I can manage something. :)

@nicolo-ribaudo
Copy link
Member

@Eric-Zayas Go for it!

@kzc
Copy link

kzc commented Nov 3, 2017

@Eric-Zayas Perhaps you could try something like this?

--- a/packages/babel-helper-remap-async-to-generator/src/index.js
+++ b/packages/babel-helper-remap-async-to-generator/src/index.js
@@ -4,6 +4,7 @@ import type { NodePath } from "@babel/traverse";
 import wrapFunction from "@babel/helper-wrap-function";
 import * as t from "@babel/types";
 import rewriteForAwait from "./for-await";
+import annotateAsPure from "@babel/helper-annotate-as-pure";
 
 const awaitVisitor = {
   Function(path) {
@@ -71,4 +72,5 @@ export default function(path: NodePath, file: Object, helpers: Object) {
   path.node.generator = true;
 
   wrapFunction(path, helpers.wrapAsync);
+  annotateAsPure(path.isDeclaration() ? path.get("declarations.0.init") : path);
 }

I didn't run the test suite due to unrelated errors in my environment.

@Andarist
Copy link
Member

Andarist commented Nov 6, 2017

@Eric-Zayas Have you found some time to work on the issue? Should we keep it as claimed for you? No rush, just let us know :)

@satya164
Copy link
Contributor

I'd like to work on this if you don't have time @Eric-Zayas

@xtuc
Copy link
Member

xtuc commented Nov 13, 2017

@Eric-Zayas FYI a PR has been merged for this. Sorry if you was working on it but you didn't answer.

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 4, 2018
@lock lock bot locked as resolved and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: perf claimed good first issue outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants