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

Add mapContextAsync #125

Merged
merged 5 commits into from
Jan 23, 2018
Merged

Add mapContextAsync #125

merged 5 commits into from
Jan 23, 2018

Conversation

stasm
Copy link
Contributor

@stasm stasm commented Jan 9, 2018

mapContextAsync asynchronously maps an identifier or an array of identifiers to the best MessageContext instance(s) from an async iterable.

The implementation is the easy part.

  • Implement mapContextAsync.
  • Write tests.

The hard part is getting our build and testing toolchain to parse for await :( Here's the list of things that need fixing:

  • Support for await in Rollup which bundles the dist files.

    The async iterators propsal is still at Stage 3. Acorn, the parser that Rollup uses, only supports Stage 4 proposals. There's a plugin which adds support for for await but it's currently not possible to extend Acorn via Rollup options. I submitted a PR to Rollup to allow adding plugins to Acorn (Allow injecting Acorn plugins rollup/rollup#1857).

  • Support for await in Babel which transpiles dist files to compat files.

    Once Rollup bundles the dist files, we use Babel to transpile it to an older version of ECMAScript. There's a transform-async-generator-functions plugin which hopefully can help Babel parse and transpile for await properly.

  • Support for await when running tests.

    We use babel-register when running tests so that node imports ES modules properly. If we continue using it for this reason, we also need Babel to parse for await properly (cf. transform-async-generator-functions from above). Or, we could try using @std/esm which is designed just for this purpose.

  • Support for await in the linter.

    Eslint also doesn't support non-Stage 4 proposals. There's babel-eslint which we might be able to use. Or, since Eslint uses Espree which is built on top of Acorn, maybe we can plug acorn-async-iteration into it somehow.

Yay, JavaScript!

@stasm
Copy link
Contributor Author

stasm commented Jan 9, 2018

Is there a chance that the Async Iteration proposal will advance to Stage 4 at the next TC39 meeting (23-25 January 2018)? All of the above would be magically solved in a matter of weeks.

@stasm
Copy link
Contributor Author

stasm commented Jan 12, 2018

An optimistic update:

Support for await in Rollup which bundles the dist files.

rollup/rollup#1857 has been approved and will be merged and released soon.

Support for await in Babel which transpiles dist files to compat files.

For some weird reason, using just the transform-async-generator-functions plugin in compat_config.js didn't work. Using the stage-3 preset did, however. We might be able to drop stage-3 and use transform-async-generator-functions when we upgrade to Babel 7.

Support for await when running tests.

I also added the stage-3 preset to .babelrc which is used by tests.

Support for await in the linter.

babel-eslint did the job.

@zbraniecki
Copy link
Collaborator

Cool! Maybe this helps: tc39/ecma262#1063

@stasm stasm force-pushed the mapContextAsync branch 2 times, most recently from bc22475 to be60997 Compare January 12, 2018 19:43
@stasm
Copy link
Contributor Author

stasm commented Jan 12, 2018

Longer term, yes, it does, thanks. Short term, I just got a green build :)

@stasm stasm requested a review from zbraniecki January 12, 2018 19:54
@stasm
Copy link
Contributor Author

stasm commented Jan 12, 2018

@zbraniecki This is ready for your review. You'll be most interested in the two last commits, but feel free to take a look at all of them. Thanks.

foundContexts[index] = context;
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be an early exit from the loop in case all contexts are found?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm correct, please add a test that verifies that contexts are not unnecessarily resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, thanks!

@stasm
Copy link
Contributor Author

stasm commented Jan 23, 2018

My PR to allow injecting Acorn plugins in Rollup has been merged today and version 0.55.0 will likely be released soon. I'll wait for it before I merge this PR so that we don't have to use my branch as a dependency in package.json.

stasm added 5 commits January 23, 2018 10:31
(For some reason the transform-async-generator-functions plugin didn't work.)
This is the async counterpart to mapContextSync. Given an async iterable of
MessageContext instance and an array of ids (or a single id), it maps each
identifier to the first MessageContext which contains the message for it.

An ordered interable of MessageContext instances can represent the current
negotiated fallback chain of languages. This iterable can be used to find the
best existing translation for a given identifier.

The iterable of MessageContexts can now be async, allowing code like this:

    async formatString(id, args) {
        const ctx = await mapContextAsync(contexts, id);

        if (ctx === null) {
            return id;
        }

        const msg = ctx.getMessage(id);
        return ctx.format(msg, args);
    }

The iterable of MessageContexts should always be wrapped in CachedIterable to
optimize subsequent calls to mapContextSync and mapContextAsync.
@stasm
Copy link
Contributor Author

stasm commented Jan 23, 2018

rollup 0.55.0 has been released. I updated the dependency to the upstream rollup repo.

@stasm stasm merged commit eb9c23e into projectfluent:master Jan 23, 2018
@stasm stasm deleted the mapContextAsync branch January 23, 2018 09:46
stasm added a commit that referenced this pull request Jan 31, 2018
  - Implement Fluent Syntax 0.5.

    - Add support for terms.
    - Add support for `#`, `##` and `###` comments.
    - Remove support for tags.
    - Add support for `=` after the identifier in message and term
      defintions.
    - Forbid newlines in string expressions.
    - Allow trailing comma in call expression argument lists.

    In fluent 0.6.x the new Syntax 0.5 is supported alongside the old Syntax
    0.4. This should make migrations easier. The parser will correctly parse
    Syntax 0.4 comments (prefixed with `//`), sections and message
    definitions without the `=` after the identifier. The one exception are
    tags which are no longer supported. Please use attributed defined on
    terms instead.

  - Add `mapContextAsync`. (#125)

    This is the async counterpart to mapContextSync. Given an async iterable
    of `MessageContext` instances and an array of ids (or a single id), it
    maps each identifier to the first `MessageContext` which contains the
    message for it.

    An ordered interable of `MessageContext` instances can represent the
    current negotiated fallback chain of languages. This iterable can be used
    to find the best existing translation for a given identifier.

    The iterable of `MessageContexts` can now be async, allowing code like
    this:

    ```js
    async formatString(id, args) {
        const ctx = await mapContextAsync(contexts, id);

        if (ctx === null) {
            return id;
        }

        const msg = ctx.getMessage(id);
        return ctx.format(msg, args);
    }
    ```

    The iterable of `MessageContexts` should always be wrapped in
    `CachedIterable` to optimize subsequent calls to `mapContextSync` and
    `mapContextAsync`.

    Because `mapContextAsync` uses asynchronous iteration you'll likely need
    the regenerator runtime provided by `babel-polyfill` to run the `compat`
    builds of `fluent`.

  - Expose the `ftl` dedent helper.

    The `ftl` template literal tag can be used to conveniently include FTL
    snippets in other code. It strips the common indentation from the snippet
    allowing it to be indented on the level dictated by the current code
    indentation.

    ```js
    ctx.addMessages(ftl`
        foo = Foo
        bar = Bar
    );
    ```

  - Remove `MessageContext.formatToParts`.

    It's only use-case was passing React elements as arguments to
    translations which is now possible thanks to DOM overlays (#101).

  - Rename `FluentType.valueOf` to `FluentType.toString1.

    Without `MessageContext.formatToParts`, all use-cases for
    `FluentType.valueOf` boil down to stringification.

  - Remove `FluentType.isTypeOf`.

    fluent-react's markup overlays (#101) removed the dependency on fluent's
    `FluentType` which was hardcoded as an import from fluent/compat. Without
    this dependency all imports from fluent are in the hands of developers
    again and they can decide to use the ES2015+ or the compat builds as they
    wish. As long as they do it consistently, regular instanceof checks will
    work well.
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

Successfully merging this pull request may close these issues.

2 participants