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

Migrate @fluent/bundle to TypeScript #436

Merged
merged 48 commits into from
Jan 23, 2020
Merged
Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
5a3ba52
WIP Rewrite to TypeScript
stasm Nov 21, 2019
b0fb0b0
Build files from esm/
stasm Nov 21, 2019
0c7adcd
Remove default exports
stasm Nov 21, 2019
dd1b458
Type custom functions
stasm Nov 21, 2019
5a44fce
FluentType, FluentBaseType
stasm Nov 21, 2019
3b010e9
RuntimePattern, RuntimeComplexPattern
stasm Nov 21, 2019
2093617
Args are unknown
stasm Nov 21, 2019
242acc1
Remove RuntimeInlineExpression type
stasm Nov 22, 2019
a804eba
Simplify the RuntimeMessage type
stasm Nov 22, 2019
3a5bf23
RuntimeTerm
stasm Nov 22, 2019
78a1aed
CustomFunction => FluentType
stasm Nov 22, 2019
cbff094
FluentBaseType<T>
stasm Nov 22, 2019
a661351
FluentArgument
stasm Nov 22, 2019
8c7650f
Emit declarations
stasm Nov 22, 2019
1536a4b
Use .js in imports
stasm Nov 28, 2019
a1b4479
Fix types in formatPattern
stasm Nov 28, 2019
3517b5a
FluentDateTime extends FluentNumber
stasm Nov 28, 2019
22c0298
Test compiled ESM files
stasm Nov 29, 2019
32a0788
make compile, make test
stasm Nov 29, 2019
847273e
noImplicitAny is implied by strict: true
stasm Nov 29, 2019
7811060
Make @fluent/dom, @fluent/react depend on @fluent/bundle for running …
stasm Nov 29, 2019
cbb7ceb
Generate API docs with typedoc
stasm Dec 2, 2019
2b958e8
Update .npmignore
stasm Dec 2, 2019
2a88b88
Remove esdoc.json
stasm Dec 2, 2019
86ce29b
Add types to package.json
stasm Dec 2, 2019
b9fad68
Fix fluent-gecko's Fluent.jsm
stasm Dec 2, 2019
6389bac
Type Scope.memoizeIntlObject
stasm Dec 2, 2019
9d54272
Revert FluentDateTime extends FluentNumber
stasm Dec 3, 2019
0e10db2
Add explicit return types
stasm Dec 9, 2019
7994277
Avoid any in FluentBundle._intls
stasm Dec 9, 2019
c9a2766
Lint using @typescript-eslint
stasm Dec 9, 2019
2429087
Message and term values are RuntimePatterns
stasm Dec 9, 2019
51725b6
Remove FluentError
stasm Dec 9, 2019
681366e
Require intl-pluralrules in tests to fix Node.js 8
stasm Dec 9, 2019
83c0c99
Call resolver functions more consistently
stasm Dec 18, 2019
dbe37dd
Drop the Runtime prefix from AST types
stasm Dec 18, 2019
cabdb9a
Remove the arity check from builtins
stasm Dec 18, 2019
6128600
Rename CustomFunction to FluentFunction
stasm Dec 18, 2019
189a996
Rename CustomTransform to FluentTransform
stasm Dec 18, 2019
fa2b5f3
Merge branch 'master' into ts-bundle
stasm Jan 10, 2020
f1f69a3
Merge branch 'master' into ts-bundle
stasm Jan 13, 2020
3eeb319
args: Array<Expression | NamedArgument>;
stasm Jan 15, 2020
af4d3b4
Move Indent into resource.ts; make it a class
stasm Jan 15, 2020
bd5ba8a
Remove FluentResource._parse
stasm Jan 15, 2020
03ea7a0
export type FluentValue = FluentType<unknown> | string;
stasm Jan 15, 2020
4d48ced
_functions = {NUMBER, DATETIME, ...functions}
stasm Jan 15, 2020
cad6b9b
Export FluentType in fluent-gecko
stasm Jan 15, 2020
3218d81
Clarify the type definition of FluentArgument
stasm Jan 15, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 11 additions & 17 deletions common.mk
Original file line number Diff line number Diff line change
Expand Up @@ -14,34 +14,28 @@ all: lint test build
# Used for pre-publishing.
dist: clean lint test build html

lint:
_lint:
@eslint --config $(ROOT)/eslint_src.json --max-warnings 0 src/
@eslint --config $(ROOT)/eslint_test.json --max-warnings 0 test/
@echo -e " $(OK) $@"
@echo -e " $(OK) lint"

html:
_html:
ifneq (,$(wildcard ./.esdoc.json))
@esdoc
@echo -e " $(OK) $@ built"
@echo -e " $(OK) html built"
endif

_clean:
@rm -f index.js compat.js
@rm -rf .nyc_output coverage
@echo -e " $(OK) clean"

deps:
@npm install
@echo -e " $(OK) $@ installed"
@echo -e " $(OK) deps installed"

depsclean:
@rm -rf node_modules
@echo -e " $(OK) $@"

CHANGELOG.md:
@if [ -z "$(SINCE)" ]; \
then echo 'Specify last version with SINCE=x.y.z' && exit 1; \
fi
@git log $(PACKAGE)@$(SINCE) HEAD --pretty=format:' - (%h) %s' $(CURDIR) \
| cat - <(echo -e "\n\n") CHANGELOG.md \
| sponge CHANGELOG.md
@echo -e " $(OK) $@ updated; make sure to edit it"

.PHONY: test html CHANGELOG.md
@echo -e " $(OK) deps clean"

OK := \033[32;01m✓\033[0m
16 changes: 0 additions & 16 deletions fluent-bundle/.esdoc.json

This file was deleted.

1 change: 1 addition & 0 deletions fluent-bundle/.gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
esm/
index.js
compat.js
4 changes: 3 additions & 1 deletion fluent-bundle/.npmignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
.nyc_output
coverage
docs
esm/.compiled
src
test
makefile
tsconfig.json
26 changes: 26 additions & 0 deletions fluent-bundle/eslint_src.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"parser": "@typescript-eslint/parser",
"parserOptions": {
"ecmaVersion": 9,
"sourceType": "module",
"project": "./tsconfig.json"
},
"env": {
"es6": true
},
"extends": [
"eslint:recommended",
"plugin:@typescript-eslint/eslint-recommended",
"plugin:@typescript-eslint/recommended",
"plugin:@typescript-eslint/recommended-requiring-type-checking",
"../eslint_src.json"
],
"rules": {
"prefer-const": "off",
"no-unused-vars": ["error", {"args": "none"}],
"@typescript-eslint/no-inferrable-types": "off",
"@typescript-eslint/no-unused-vars": ["error", {"args": "none"}],
"@typescript-eslint/no-use-before-define": "off",
"@typescript-eslint/explicit-function-return-type": "error"
}
}
45 changes: 34 additions & 11 deletions fluent-bundle/makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,57 @@ GLOBAL := FluentBundle

include ../common.mk

build: index.js compat.js
lint:
@eslint --config ./eslint_src.json --max-warnings 0 src/*.ts
@eslint --config $(ROOT)/eslint_test.json --max-warnings 0 test/
@echo -e " $(OK) lint"

test:
@nyc --reporter=text --reporter=html mocha \
--recursive --ui tdd \
--require esm \
--require ./test/index \
test/**/*_test.js
compile: esm/.compiled

esm/.compiled: $(SOURCES)
@tsc
@touch $@
@echo -e " $(OK) esm/ compiled"

build: index.js compat.js

index.js: $(SOURCES)
@rollup $(CURDIR)/src/index.js \
index.js: esm/.compiled
@rollup $(CURDIR)/esm/index.js \
--config $(ROOT)/bundle_config.js \
--banner "/* $(PACKAGE)@$(VERSION) */" \
--amd.id $(PACKAGE) \
--name $(GLOBAL) \
--output.file $@
@echo -e " $(OK) $@ built"

compat.js: $(SOURCES)
@rollup $(CURDIR)/src/index.js \
compat.js: esm/.compiled
@rollup $(CURDIR)/esm/index.js \
--config $(ROOT)/compat_config.js \
--banner "/* $(PACKAGE)@$(VERSION) */" \
--amd.id $(PACKAGE) \
--name $(GLOBAL) \
--output.file $@
@echo -e " $(OK) $@ built"

.PHONY: test
test: esm/.compiled
@nyc --reporter=text --reporter=html mocha \
--recursive --ui tdd \
--require esm \
test/**/*_test.js

html:
@typedoc src \
--out ../html/bundle \
--mode file \
--excludeNotExported \
--excludePrivate \
--logger none \
--hideGenerator
@echo -e " $(OK) html build"

clean:
@rm -f esm/*.js esm/*.d.ts esm/.compiled
@rm -f index.js compat.js
@rm -rf .nyc_output coverage
@echo -e " $(OK) clean"
Expand Down
7 changes: 3 additions & 4 deletions fluent-bundle/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@
"email": "stas@mozilla.com"
}
],
"directories": {
"lib": "./src"
},
"type": "commonjs",
"main": "./index.js",
"module": "./src/index.js",
"module": "./esm/index.js",
"types": "./esm/index.d.ts",
"repository": {
"type": "git",
"url": "https://github.com/projectfluent/fluent.js.git"
Expand Down
80 changes: 80 additions & 0 deletions fluent-bundle/src/ast.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
export type Message = {
id: string;
value: Pattern | null;
attributes: Record<string, Pattern>;
};

export type Term = {
id: string;
value: Pattern;
attributes: Record<string, Pattern>;
};

export type Pattern = string | ComplexPattern;

export type ComplexPattern = Array<PatternElement>;

export type PatternElement = string | Expression;

export type Expression =
| SelectExpression
| VariableReference
| TermReference
| MessageReference
| FunctionReference
| Literal;

export type SelectExpression = {
type: "select";
selector: Expression;
variants: Array<Variant>;
star: number;
};

export type VariableReference = {
type: "var";
name: string;
};

export type TermReference = {
type: "term";
name: string;
attr: string | null;
args: Array<Expression | NamedArgument>;
};

export type MessageReference = {
type: "mesg";
name: string;
attr: string | null;
};

export type FunctionReference = {
type: "func";
name: string;
args: Array<Expression | NamedArgument>;
};

export type Variant = {
key: Literal;
value: Pattern;
};

export type NamedArgument = {
type: "narg";
name: string;
value: Literal;
};

export type Literal = StringLiteral | NumberLiteral;

export type StringLiteral = {
type: "str";
value: string;
};

export type NumberLiteral = {
type: "num";
value: number;
precision: number;
};
54 changes: 0 additions & 54 deletions fluent-bundle/src/builtins.js

This file was deleted.

61 changes: 61 additions & 0 deletions fluent-bundle/src/builtins.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* @overview
*
* The FTL resolver ships with a number of functions built-in.
*
* Each function take two arguments:
* - args - an array of positional args
* - opts - an object of key-value args
*
* Arguments to functions are guaranteed to already be instances of
* `FluentValue`. Functions must return `FluentValues` as well.
*/

import {
FluentValue,
FluentNone,
FluentNumber,
FluentDateTime
} from "./types.js";

function values(opts: Record<string, FluentValue>): Record<string, unknown> {
const unwrapped: Record<string, unknown> = {};
for (const [name, opt] of Object.entries(opts)) {
unwrapped[name] = opt.valueOf();
}
return unwrapped;
}

export function NUMBER(
args: Array<FluentValue>,
opts: Record<string, FluentValue>
): FluentValue {
let arg = args[0];

if (arg instanceof FluentNone) {
return new FluentNone(`NUMBER(${arg.valueOf()})`);
}

if (arg instanceof FluentNumber || arg instanceof FluentDateTime) {
return new FluentNumber(arg.valueOf(), { ...arg.opts, ...values(opts) });
}

throw new TypeError("Invalid argument to NUMBER");
Copy link
Contributor Author

@stasm stasm Dec 3, 2019

Choose a reason for hiding this comment

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

Unfortunately, the amount of changes in this file makes diff think it's a new file. Here's what NUMBER used to look like:

export function NUMBER([arg], opts) {
  if (arg instanceof FluentNone) {
    return new FluentNone(`NUMBER(${arg.valueOf()})`);
  }

  let value = Number(arg.valueOf());
  if (Number.isNaN(value)) {
    throw new TypeError("Invalid argument to NUMBER");
  }

  return new FluentNumber(value, { ...arg.opts, ...values(opts) });
}

This code didn't compile in TS because arg.opts doesn't exist on FluentType; it's only defined on FluentNumber and FluentDateTime. BTW the JS code works because {...undefined} evaluates to {}.

In the TS rewrite, I introduced explicit arg instanceof FluentNumber || arg instanceof FluentDateTime guards and dropped the Number() parsing. This changes the current behavior of @fluent/bundle: calling NUMBER on a string-typed variable used to parse the string as a number and format it. After this change, the argument will be rejected as invalid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1605062 to prepare for this change in mozilla-central.

}

export function DATETIME(
args: Array<FluentValue>,
opts: Record<string, FluentValue>
): FluentValue {
let arg = args[0];

if (arg instanceof FluentNone) {
return new FluentNone(`DATETIME(${arg.valueOf()})`);
}

if (arg instanceof FluentNumber || arg instanceof FluentDateTime) {
return new FluentDateTime(arg.valueOf(), { ...arg.opts, ...values(opts) });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FluentDateTime extends FluentNumber now, which made this possible. I needed a way to type-guard the access to .opts, which is only guaranteed to exist on FluentNumber and FluentDateTime. Initially I just made two instanceof checks here, but then decided that FluentNumber and FluentDateTime were close enough to make them related. FluentDateTime keeps its value as a number, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm having second thoughts about this.

In TypeScript, this works because Intl.DateTimeFormatOptions happens to be assignable to Intl.NumberFormatOptions. I'm not sure if we should rely on this.

On a higher level, this is a spec question: what should be the outcome of NUMBER($date) and DATETIME($number)? I think it will be safer to not make assumptions about the answer right now and wait for the formatting spec to be finished.

I'll revert the FluentDateTime extends FluentNumber change.

}

throw new TypeError("Invalid argument to DATETIME");
}
Loading