Skip to content

regeneratorRuntime required with @fluent/bundle even when not using the compat build #497

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

Closed
elisehein opened this issue Jun 17, 2020 · 6 comments

Comments

@elisehein
Copy link

elisehein commented Jun 17, 2020

Hello!

I'm not sure if this is me merely misunderstanding the docs, but I thought I'd raise anyway.

The README for @fluent/bundle mentions that if using the compat build, regenerator runtime must also be provided.

I'm seeing regeneratorRuntime required even though I'm not using the compat build.

Minimal working example:

package.json dependencies

"@babel/core": "^7.10.2",
"@babel/preset-env": "^7.10.2",
"@fluent/bundle": "^0.15.1",
"@fluent/react": "^0.12.0",
"babelify": "^10.0.0",
"browserify": "^16.5.1",
"react": "^16.13.1"

index.js

import { FluentBundle, FluentResource } from "@fluent/bundle";
import { ReactLocalization } from "@fluent/react";

function *generateBundle(locale) {
  const bundle = new FluentBundle(locale);
  bundle.addResource(new FluentResource("hello = Hola!"));
  yield bundle;
}

export default function l10n(locale) {
  return new ReactLocalization(generateBundle(locale));
}

l10n("en-GB");

index.html

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8">
  </head>

  <body>
    <script src="app.js"></script>
  </body>

</html>

Transpilation step

> node_modules/.bin/browserify index.js -t [ babelify --presets [ @babel/preset-env ] ] > app.js

Browser runtime error

Screenshot 2020-06-17 at 10 13 48

I can see regeneratorRuntime in the transpiled output:

var _bundle = require("@fluent/bundle");

var _react = require("@fluent/react");

var _marked = /*#__PURE__*/regeneratorRuntime.mark(generateBundle);

function generateBundle(locale) {
  var bundle;
  return regeneratorRuntime.wrap(function generateBundle$(_context) {
  // ...

This is easily fixed by me adding import regeneratorRuntime from "regenerator-runtime"; to the top of my file. But I suspect this isn't what I should be doing. Is it a case of out of date documentation?

@stasm
Copy link
Contributor

stasm commented Jun 17, 2020

Thank you for a comprehensive issue report!

In #472 I removed the compat builds altogether and in #483 I removed the mention of regeneratorRuntime from the README. I was planning to continue the build system cleanups in #480 but I hit a roadblock. I needlessly put off releasing new versions of all @fluent packages until I can figure how to fix it. I'm sorry about that, I'll work on releasing new versions next week.

The main motivation for removing the compat builds in #472 was that today's build tools are capable of transpiling dependencies which means we should be able to provide a single version of a @fluent package written in fairly modern JS, and the build tool will transpile it if needed.

In your example, I assume that browserify is transpiling all code, including the @fluent/bundle dependency. So even if you're not using the compat build, you end up with a transpiled version of @fluent/bundle anyways; hence the need for the regeneratorRuntime. Am I understanding this right?

@elisehein
Copy link
Author

elisehein commented Jun 17, 2020

In your example, I assume that browserify is transpiling all code, including the @fluent/bundle dependency. So even if you're not using the compat build, you end up with a transpiled version of @fluent/bundle anyways; hence the need for the regeneratorRuntime. Am I understanding this right?

I'm indeed using browserify, but as far as I'm aware, the transforms (babelify with @babel/preset-env) do not automatically apply to files in node_modules, so the expectation is that any packages imported from node modules are already transpiled, and are just included in the bundle as-is. So that still leaves me a bit confused about regeneratorRuntime popping up in the transpiled code...

@Pike
Copy link
Contributor

Pike commented Jun 17, 2020

About that piece of doc, IIRC that was supposed to mean:

If you're not using the compat builds, you need a fancy new browser, and anything just slightly old will break in all kinds of ways.
If you want older browsers, the compat builds get you far, but we didn't include the regenerators, as you might need them elsewhere, and then your transpiled js would get unnecessarily large.

So you need generators either way, but the non-compat builds break on a lot more than just that.

@elisehein
Copy link
Author

@Pike Thanks, that makes sense!

@stasm
Copy link
Contributor

stasm commented Jun 17, 2020

so the expectation is that any packages imported from node modules are already transpiled, and are just included in the bundle as-is. So that still leaves me a bit confused about regeneratorRuntime popping up in the transpiled code...

Could it be that browserify (or babelify?) uses the regeneratorRuntime to transpile the *generateBundle(locale) generator function in your index.js?

In fact, @fluent/bundle doesn't even use yield nor async functions; even transpiled, it wouldn't require the runtime to be present for legacy browsers. OTOH the runtime would be needed for transpiled versions of packages like @fluent/dom and @fluent/react.

@elisehein
Copy link
Author

elisehein commented Jun 17, 2020

@stasm Indeed, thanks, that looks like it's the crux of it!

To test, when I replace *generateBundle with a vanilla JS function that only pretends to return a generator, I no longer need to import regeneratorRuntime:

function generateBundle(locale) {
  const bundle = new FluentBundle(locale);
  bundle.addResource(bundle.addResource(new FluentResource("hello = Hola!"));
  let result = {};
  result[Symbol.iterator] = () => {
    return {
      next: () => {
        return {
          done: true,
          value: bundle
        };
      }
    };
  };

  return result;
}

I'll close this issue now, thanks all for helping me investigate.

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

No branches or pull requests

3 participants