Closed sebpiq closed 5 years ago
:+1: Would love to use the evaluator but don't want to load everything else that I won't use.
Realise that this is an old thread but this would be feasible if mathjs were to be composed from ES Modules instead of using the require
function.
// old
const math = require('mathjs');
console.log(math.eval('2 + 2'));
// new
import { evaluate } from 'mathjs'; // eval is a reserved word!
console.log(evaluate('2 + 2'));
Thanks for your input. ES modules would definitely help, with that you could easily load only specific functions instead of the whole library. There is another dimension though that we cannot easily solve this way: being able to include/exclude specific data types: maybe you're not interested in using Matrices or BigNumbers. Each math.js function contains an implementation for each of the data types, and it's not so trivial to split this up.
Note that since this topic was opened, math.js is already changed into a much more modular library, and there are already ways to just include the features that you need: http://mathjs.org/docs/custom_bundling.html
I would love to bring this modularity a step further though.
Yeah, I guess this also depends on how stateful you want a modular version of mathjs. Currently the module looks very stateful; math.eval
especially has a lot of side-effects on (repeated) invocation. I'm sure there's a way to supply each implementation with functions for the different data types though, if you want to go down that route. 😄
const types = {
// this could be from an es module
'Array, any': function (x, y) {
// use matrix implementation
return algorithm14(matrix(x), y, subtract, false).valueOf();
},
};
function subtractFactory (types) {
return function subtract (a, b) {
return typed('subtract', types);
}
}
export default subtractFactory;
Not sure if this is as straightforward as that, but it's just an idea.
I don't yet understand your idea, but I'm totally open for good ideas here though: the current solution with factory functions works but I find it too complicated, and it doesn't allow you to do tree shaking on specific types.
Right, so let's try and take a simple transformation, the square of a number. The current implementation means that tree-shaking cannot be done because the square transformation has to account for all of the different permutations of types. However, my idea was to be able to supply the square function with an object of types.
In this implementation, instead of injecting the typed
function as a parameter to the factory function, we load it from another file. I'm not familiar with the code so I don't know if you would still want to be able to customise this, but that can be done too. We then also export all of the methods necessary which a consumer of this module needs.
import typed from '../typed';
export function squareNumber (x) {
return x * x;
}
// rest of the methods
export function square (config) {
return typed('square', config);
}
export const squareConfig = {
'number': squareNumber,
// rest of the methods
};
Now, a consumer of this module can supply their own typed
config, so now they can customise the behaviour should they choose to. This function will only be able to validate number types, so all of the other validators can be excluded from bundled code:
import {square, squareNumber} from 'mathjs';
const squared = square({'number': squareNumber})(2); // 4
They can also supply the defaults to get the full list, now all methods would be bundled.
import {square, squareConfig} from 'mathjs';
const squared = square(squareConfig)(2); // 4
I think then you would also be able to provide a wrapped method which would do the above and this would be removed from the tree if it were unused;
import {defaultSquare} from 'mathjs';
const squared = defaultSquare(2); // 4
That's an interesting idea, thanks! That could work. So instead of exporting a single function from a file, each file should export a function for every signature, and we have to to compose them later on.
There is some difficulty in that the functions must be created via a factory function first, this factory function passes configuration and some more things. So every function signature would have to be wrapped in it's own factory function which is then exported. That will give a lot of overhead.
I think this is worth an experiment, see where we end up and what difficulties we encounter.
(btw sorry for the late reply)
It has been a while but it's getting more and more important to make mathjs fully modular. I want to propose a change in the core architecture of mathjs, based on the suggestion of @ben-eb. I've also implemented a proof of concept in the branch modular-architecture
.
There are two main issues why we want to change the architecture:
pow
uses multiply
, but when you replace multiply
with an extended version of the function (for example with support for a new data type), pow
still uses the original, built-in version. And the other way around: if you are just need numbers and not BigNumbers and fractions, you would like to replace multiply
with a lightweight version just handling numbers, which is not possible right now.I have been thinking about how to change the archicture such that we can make mathjs fully modular. Sort of like a step from the monolith library underscore
to the modular lodash
. What we need is:
The current factory functions looks like:
function factory (type, config, load, typed) {
const multiply = load(require('./multiply'))
// ...
return // ... implementation of pow, normally a typed-function
}
exports.name = 'pow'
exports.factory = factory
This factory function can only easily be used by math.import
for example because of the "magic" load
function provided by math.import
.
My idea for a new dependency injection architecture is:
export function createPow(math) {
assertDependencies(this.dependencies, math)
return // ... implementation of pow, normally a typed-function
}
createPow.dependencies = ['multiply', ...]
Constructing such a function is more straight forward, it doesn't require a special math.import
function:
const math = {}
math.multiply = createMultiply(math)
math.pow = createPow(math)
// ... now you can use math.pow
This way, you can load your own version of multiply
(either a more extensive version or a light weight version), and this version is dynamically bound, so pow
will automatically use the new version of multiply
when replacing it in math
.
A drawback is that calling createPow
will fail if math
doesn't contain the required dependencies (multiply
in this case). This is because now the caller is responsible for the dependencies instead of the function itself. By exposing createPow.dependencies
, we can write a function math.import
which is at least capable of loading a list with functions in the right order (multiply
first, pow
next).
Importing and composing your own functions may look like:
import { create } from 'mathjs'
import * as allNumberFns from 'mathjs/number'
import * as arithmeticBignumberFns from 'mathjs/bignumber/arithmetic'
import add as fractionAdd from 'mathjs/fraction/add'
// the imports are usable as is, though they are no typed functions
// importing them into math using `import`
const math = create()
math.import([allNumberFns, arithmeticBignumberFns, fractionAdd])
// now you have functions like `add` supporting numbers and bignumbers
There are two types of functions:
math.add
.math.pow
which depends on math.multiply
and others, and which accpets number, BigNumber, and Fraction as input. Generic functions will be created via a factory function to get all dependencies injected.It will be nice if we can expose these low level functions so they can be used on their own, or can be used to mix and match high level functions.
There is one special case here: low level functions may require config, which needs to be passed via dependency injection. A solution is to pass config
as an optional extra parameter to these functions, for example:
function equal(a, b, config) {
const epsilon = config ? config.epsilon : DEFAULT_EPSILON
return // ...
}
For all functions this is a suitable solution, only functions like acos
and asin
require some more thought: the number implementation may return a number or a Complex number depending on the input value and config. Maybe we simply have to provide a low level number implementation and a high level number implementation.
Explicit low level functions, should also have input types defined. This could be done by creating them as a typed-function
:
import * as typed from 'typed-function'
export add = typed('add', {
'number, number': function (a, b) {
return a + b
}
})
but this introduces an abstraction layer again (it's not low level anymore), and it gives quite some overhead in the code. Nicer is to attach type information as a property on the function. This way the function can both be used as is, or can be turned into a typed-function
by typed-function
itself or by math.import
:
export function add(a, b) {
return a + b
}
add.signature = 'number, number'
Usage:
// plain JS usage, no types
const c = add(a, b)
// turn into typed function:
const addTyped = typed(add)
const d = addTyped(a, b)
More than one third of the high level functions do not need dependencies, and are simply an aggregation of a number of low level functions into a typed-function
. Examples are abs
, addScalar
, sin
, etc.
Enclosing them in a factory function is unnecessary overhead. I think though we should keep them as such because of:
.toTex
properties there (this could be changed)With the new modular structure, people simply can compose functions themselves, so if needed there already is a way to work around this overhead and not have it end up in your bundle.
The file structure will have a clear distinction between explicit (low level) and generic (high level) functions. It can look like:
// high level functions having the same stucture they have now:
src/arithmetic/add.js // containing generic implementation of add
src/arithmetic/multiply.js // containing generic implementation
// low level functions:
src/number/arithmetic.js // containing all number implementations like add, subtract, multiply
If necessary, we can still put all explit, low level functions in a separate file, but they are probably very small and will only cause unnecessary overhead.
For easy consumption of individual functions, we could create a build script which creates shortcuts to individual functions into the root of the project, similar to lodash
, so that you can do imports of individual functions in various ways:
// high level (only efficient when the bundler supports tree shaking)
import { create, add, multiply } from 'mathjs'
const math = create()
math.import(add, multiply)
math.multiply(2, 3) // 6
// high level
import create from 'mathjs/create'
import multiply from 'mathjs/multiply'
const math = create()
math.import(multiply)
math.multiply(2, 3) // 6
// low level
import { multiply } from 'mathjs/number/multiply'
multiply(2, 3) // 6
Since high level functions need a to be created via factory functions (they need config
and dependencies), they must be loaded via math.import
. Low level functions can be used directly, or if wanted can be loaded via math.import
too.
Important here is that you don't have to remember in which group a function is located (like arithmetic
) and also that you don't need to use .../lib/...
in the url.
I think the refactoring will take quite some effort, and we need to keep the old solution working whilst migrating piece-by-piece to the new architecture. We must to keep the old type of factory functions working in math.import
anyway to keep mathjs backward compatible, and we must keep the same file structure for the generic functions to to break things.
I've worked out an experiment in the branch modular_architecture
.
Demonstrating how it can be used: examples/advanced/custom_function_composition.js examples/advanced/custom_function_loading.js
Some explicit (low-level) functions: src/number/arithmetic.js src/bignumber/arithmetic.js
A refactored generic (high-level) function hypot
:
src/function/arithmetic/createHypot.js
I'm really exited about finally addressing this issue, and this first proof of concept looks really promising.
Please let me know what you think!
EDIT 2018-09-15: Updated section "File structure", high level functions cannot directly be imported via ES6, they have to be imported/created too since they are factory functions.
I like this proposal a lot. :+1:
In this part:
export function createPow(math) {
assertDependencies(this.dependencies, math)
return // ... implementation of pow, normally a typed-function
}
createPow.dependencies = ['multiply', ...]
I worry that is it too easy for someone to miss dependencies. I could see my self doing something like:
export function createPow(math) {
assertDependencies(this.dependencies, math)
return function (base, exponent) {
return math.multiply(math.add(base, 5), exponent)
}
}
createPow.dependencies = ['multiply']
Above, math.add
is used but not listed as a dependency. 99% of the time, createPow
will be called with the normal math object and so the assertion will not fail. I would like to suggest something like:
export function createPow(math) {
const { multiply, add } = getDependencies(this.dependencies, math)
return function (base, exponent) {
return multiply(add(base, 5), exponent)
}
}
createPow.dependencies = ['multiply', 'add']
I envision the getDependencies
function returning an object containing only the functions listed in the array of dependancies. This makes it much harder for folk to use a function without listing it as a dependency.
Also I found a typo:
Exlicit (low level) vs generic (high level) functions
Exlicit -> Explicit
Thanks for your feedback @harrysarson !
I worry that is it too easy for someone to miss dependencies.
Yes I think this will definitely happen and we should should catch this with unit tests or when loading the function.
Your suggestion for an getDependencies(...)
is interesting. Still, people can accidentally use math.add
instead using the results from getDependencies
.
I was thinking in a different direction here: in the unit tests we could create a util function to pass only the listed dependencies to createPow
. Or even better: we could also let math.import
pass only the listed dependencies. I think that will immediately break unit tests when forgetting to list a dependency.
The purpose of the assertDependencies
is just to give proper error messages at load time rather than getting vague error messages later on when using the created function.
There is an other important topic around using const { multiply, add } = math
(which is very convienient and gives more concise code!): The dependencies are bound at load time, which means that if you replace add
with a new version (supporting new data types or something), pow
still does use the old version of add
. Two solutions to this problem: (a) bind dynamically using math.add
, or (b) after changing a function, re-load all functions that depend on this function. Maybe the latter is a better option, and it is easy to automate. Also, we should check whether dynamic binding has a negative effect on the performance or not. Thinking about it, I quite like solution (b).
I've fixed the "Exlicit" typo, thanks.
At the risk of adding to the noise here, the only thing stopping me from using mathjs at the moment is also the fact that it ends up including way more code than I need (so I just end up copying out the parts I want). Maybe instead of a having a function that gets the dependencies - which would add a runtime overhead (which isn't the best option in my opinion), why not just let the tree shakers take care of this sort of thing?
You could just switch to using es6 imports in all of your files, and all of the dependencies would need to be included to pass the unit tests, then if a user wanted to actually use a particular function, they could just do:
// User script
import { simplify } from 'mathjs/algebra'
Inside algebra, you'd include all of the dependencies required to run it, so:
// Algebra.js
import { dependencyA } from 'somewhere'
import { dependencyB } from 'another'
// ...
export function simplify () {
}
So your tree shaker could just go to work killing off anything it doesn't need. Of course, this would mean using a new file to just export a bunch of stuff, sort of like the one d3 uses; but this would make it so much easier for people to rip out only a small part of the library.
Thanks for your input @saivan . Making mathjs tree-shake-friendly is exactly what we aim for.
It's not as easy as for example the functions of lodash. The difficulty is that most of the mathjs functions use a global configuration, which means they have to be created via a factory function.
So we will come quite close to your proposal, except you have to load the imported functions into mathjs after importing, something like this (a copy of an example from my proposal):
// high level (only efficient when the bundler supports tree shaking)
import { create, add, multiply } from 'mathjs'
const math = create()
math.import(add, multiply)
math.multiply(2, 3) // 6
If you know a different solution to the config problem I would love to hear it. I have been thinking about passing config as a last, optional argument. I don't see this work nicely in practice from an end-user point of view though.
Well, maybe hiding this from the user would be possible too. By having a global factory function that is implicitly created like this:
// math.js
// Make the create class
class create () {
define ( name, fn ) {
}
get ( name ) {
}
}
// Then make an instance
export const math = create()
You could then pull it in, inside another file like the add file and define the add function in there:
// add.js
// Get the math factory from the other file
import { math } from "math.js"
// Define the function you want to add
math.define('add', () => {
})
// Export the function for the user to use
export const add = math.get('add')
Then the user could just do:
// User code
import { add } from "mathjs/add"
This would automatically create the factory function once, and then it would just give them back the single function they asked for.
This syntax is not polished at all, but just an idea.
@saivan yes you are right. Thinking about it, this is basically how mathjs works right now already: when doing import * as math from 'mathjs'
you get an instance with a global config which would be shared by any other component in the same application that uses mathjs too. Maybe it would be better to disable changing this global config (it is changable right now), since it can mess up functioning of other parts of an application also using mathjs.
At least, we could indeed do it like this and it would work with tree shaking and a global config out of the box, with the high-level functions containing support for all datatypes.
I don't have a clear picture how we could combine this in a consistent and easy to use way with these other goals of custom configuration and support for just a few data types instead of all. The charming thing of the solution where you're forced to create a mathjs instance yourself is that though a bit verbose, it's really simple to understand and it's simple to mix and match everything you want, and you don't have any global config that can be changed by other components. It's similar to having to create an express js server after you've imported it, or similar to how you create a custom fontawesome bundle.
We could make it a little bit less verbose like:
import { create, add, multiply } from 'mathjs'
const math = create(create, add)
math.multiply(2, 3) // 6
and we can create "shortcuts" for all kind of combinations of functions:
import * as all from 'mathjs/all'
import * as arithmetic from 'mathjs/arithmetic'
import * as pow from 'mathjs/arithmetic/pow'
import * as numberArithmetic from 'mathjs/number/arithmetic'
// ...
const math = create(...)
It may be interesting to have something like import * as sqrt from 'mathjs/arithmetic/pow'
return not just the (factory) function pow
but also all it's dependencies like multiply
, so after you do
const math = create(pow)
it just works without you having to worry about the dependencies of pow
. I think we can automatically generate such index files like 'mathjs/arithmetic/pow'
.
Or, thinking aloud, have pow
in import {pow} from 'mathjs'
return a list with factory functions which will load pow
and it's dependencies into math when calling math.create(pow)
so you don't need a ton of index files like mathjs/arithmetic/pow
.
I've been thinking about it and I think it will be quite easy to enable user code like
import { add } from "mathjs
, if we indeed use a default config like we have now. It's simpler than I thought, I really have to get used to not having this central "math" namespace but only have functions tight to other functions, their dependencies, and config is just one of these dependencies.
The index.js file that is loaded when doing import ... from 'mathjs'
can look something like this, and three shaking should work out of the box:
// import factory functions
import createAdd from 'mathjs/arithmetic/add'
import createMultiply from 'mathjs/arithmetic/multiply'
import createPow from 'mathjs/arithmetic/pow'
// create instances
export const add = createAdd({ ... })
export const multiply = createMultiply({ ... })
export const pow = createMultiply({ add, multiply, ... })
Still thinking about how creating a new math instance (the current math.create
) can work out.
Sorry I didn't respond to your last message. I totally missed it in my feed. I actually like this proposal a lot. But how about also having the createAdd
function automatically created in the mathjs/arithmetic/add
section with a set of reasonable parameters that the user is likely to use. So they have:
// file: mathjs/arithmetic/add
export function createAdd( ...params ) {
}
export function add = createAdd ( ...sensibleDefaults )
This would be really flexible for folks who don't necessarily need to craft their own version of this. I'm just thinking that if I were to import a slew of functions, I wouldn't want to initialise them all. That could potentially get hairy for the end user.
If they don't import add, I'm not sure whether the tree-shakers would realise that they don't need to include that code, but I feel like they would. I can't be sure though.
Ah, that's an interesting idea @saivan, I will keep that in mind!
What I was thinking about right now is creating the function instances in an "index" file, separate from the factory functions. I want to create a couple of index files which are usable right away, created with "sensible defaults", like:
import { add } from 'mathjs'
add(2,3) // supports any data type
import { add } from 'mathjs/number'
add(2,3) // supports only numbers,
// so very small (no BigNumber, Complex, Unit Matrices, etc.)
import { add } from 'mathjs/bignumber'
add(2,3) // supports only BigNumbers
If possible I would like to have flat index files, so you don't have to remember/know that add
is located under mathjs/arithmetic/add
. For the factory functions it makes sense to be able to load them individually or load all functions of one category.
Let me see how it works out. I plan working on the migration in two weeks from now, really looking forward to work on it. Will keep you posted and probably ask for feedback along the road if you don't mind.
Happy to take a look of course! You could also avoid flat files by just having another file whose only purpose is to import and export parts. For example, you could do:
/// math.js
export const matrix = Object.assign({},
import("...somepath/inverse"),
import("...somepath/eigenvalues"),
...
)
export const arithmetic = Object.assign({},
import("...somepath/add"),
import("...somepath/subtract"),
...
)
Then if the user imported import * as m from 'mathjs'
, they would have access to (at least) this object:
mathjs = {
matrix: {
inverse: fn(),
eigenvalues: fn(),
},
arithmetic: {
add: fn(),
sub: fn(),
},
..
}
But then if they just want particular files, they could use this import file as a guide to make their own smaller version of the library :)
Thanks. There are indeed quite some options for importing/exporting. I have to see and experiment a bit what works out best in the case of mathjs. But we will definitely end up with some index files just piping imports/exports I think.
I look forward to this. Very exciting!
I'm working on the refactoring into a modular structure :tada: . It works out quite nicely, though it was quite a puzzle to get it set up such that I can migrate functions one by one without having a totally broken code base for a long time. Right now all unit tests still work, the bundle is incomplete, and loading is slower because I turned off lazy loading during the migration.
I've now converted all arithmetic functions and some more. I've introduced a helper function factory
to make creating a factory function easy, readable, and robust (it does check dependencies for you, that helped me a lot already). They look like:
import { factory } from '../../utils/factory'
const name = 'log'
const dependencies = ['config', 'typed', 'divideScalar', 'type.Complex']
export const createLog = factory(name, dependencies, ({ typed, config, divideScalar, type: { Complex } }) => {
// ...
})
You can have a look at the converted functions here: https://github.com/josdejong/mathjs/tree/modular_architecture/src/function/arithmetic
I'm working on two new "index" files: mainAll.js
containing the full functions, and mainNumber.js
which will contain the lightweight number versions of all functions:
https://github.com/josdejong/mathjs/blob/modular_architecture/src/mainAll.js
https://github.com/josdejong/mathjs/blob/modular_architecture/src/mainNumber.js
In the folder test-tree-shaking
I'm working out a number of use cases, to see how they work from a user perspective and to validate whether tree shaking works. Not all of them are functional, and tree shaking doesn't yet work everywhere (presumably because some files still have side effects). See:
https://github.com/josdejong/mathjs/tree/modular_architecture/test-tree-shaking
Please let me know if you have any feedback!
All functions are now converted :tada: . Next is to work out the new index files now and start using them, get tree-shaking actually working, and figure out why the unit tests are much slower now.
Babel is probably the culprit there - mocha has to transpile all of the code; this is slow. Perhaps moving tests to headless chrome might speed things up (alot) 😁
It's hard to say. I've done some experiments and profiling, it looks like createTypedFunction
is three times as slow as it used to be, which is odd since I still use the same version of typed-function
with the same types and conversion as before the refactoring. Maybe the lazy loading doesn't work as it should or something like that. Still puzzled.
Is it only running slower during testing, or is that also something you're seeing in the browser?
The bundled version consistently loads twice as slow on both Chrome and Firefox. The unit tests run four times as slow.
Oh wow. Thats pretty severe. I think the profilers your best friend there; but I'm sorry I can't say anything helpful without actually digging through the code
Yes, I've done a bit of profiling already and have some ideas to test further. At least I'm glad that the behavior is consistent and reproducible :)
I do understand now what's going on with the performance issue: lazy loading is indeed not working. The reason is that in the new index file, I create instances of all functions and export them one by one. The lazy loading mechanism works for getting properties from an object (i.e. math
), and not when exporting individual functions as I do now.
A solution is to wrap all functions in a function which does lazy loading. That would mean some overhead for every function call, and a more complicated setup. Doesn't feel like the right approach.
An other issue I'm encountering is that tree shaking and exporting individual functions conflicts with wanting a namespace (math
) containing all functions, where lazy loading makes sense. There are a few special functions like parse
and chain
, which require to have a namespace containing all available functions. Instead of binding this on construction, we could also pass math
as an argument (and allow currying of the function to stay practical).
I think I will start making two separate index files for these two cases, and get that working. After that I can see if they can be combined somehow in a single index file (which should allow tree shaking and should not be confusing to understand).
I'm making nice progress with the refactoring. I was struggling with the configuration which is needed in the functions. I think the best approach is to make the functions pure+immutable: we create them once by passing dependencies (including config) to their factory function, and after that they don't change. To change the config, you have to create the functions again. The alternative (the old situation) is that some functions have to listen to changes in config, constants (like pi) have to update themselves, etc. which works when having a single namespace with event listeners, but totally doesn't work when you want to export individual, standalone functions.
Currently the unit tests are very slow, I know the reason, it's a WIP.
Currently all use cases work :tada: . I'm not really happy with how intuitive everything works, and there are too many different ways to do things so maybe we should simply limit to a few of the most relevant options and offer that without confusing users. Feedback welcome!
The tree shaking should work by now but doesn't in the most important cases (like usecase4). I'm still looking into it. It almost looks like there is a bug in webpack tree shaking or something.
Tree shaking is now starting to work, using a workaround. It turns out webpack is not able to tree-shake functions which are the result of a factory function, see https://github.com/webpack/webpack/issues/4784#issuecomment-443415648. The workaround is to add a comment /* #__PURE__ */
for all functions. Still, the bundles are larger then strictly necessary, I guess I have to add more of these pure annotations everywhere in the code. Not ideal but for now acceptable.
yippee, finally fixed the unit tests being slow, they are as fast as they where before.
I'm now working on refactoring the parser now. There is a structure in place which has the parser work with one-based numbering, whilst the separate functions are zero-based. This is achieved with "transform" functions, a special structure in the math.import
function, and two versions of the imported functions (zero-based functions on math
, one-based functions on math.expression.mathWithTransform
). This structure is quite complicated, and doesn't fit nicely with the new setup (pure functions, flat dependency list).
I was thinking: things would become much simpler when, instead of the transforms solution, we would create a configuration option { indexNumbering: 0 | 1 }
. This would be a serious breaking change, but I think for the better as it makes the library simpler to understand and more consistent.
If anyone has ideas or arguments in favor or against this idea please share it here.
My 2 cents:
Having index numbering configurable could introduce subtle bugs when merging code that assumes different indexing together.
I believe that consistency is the most important thing here.
I'm indeed in search of a consistent solution, with as little magic involved as possible. Less than we had.
By pulling the functions apart to allow for tree-shaking, there is no "magic" environment which can provide the functions parse
and eval
with a different (one-based) version of some specific functions like filter
and forEach
. In the old solution, eval
basically used a one-based mathWithTransform
namespace whilst other functions where using the zero-based math
namespace. Functions like filter
and forEach
had two versions (a zero-based and a one-based "transform" version) which where "magically" loaded in parallel.
In the new solution, creating an eval function boils down to const evaluate = createEvaluate({ config, math })
where math
is an arbitrary namespace containing all functions you want available. In one way or another, the user has to decide to pass a set of zero-based functions or one-based functions, but we can't enforce this. Therefore I am looking into alternative solutions that are easy to understand, like having just one filter
function but be able to create it zero-based or one-based depending on a single config value.
I'm open to any suggestions!
I've given this more thought. I think it's not a good idea after all to make something as core as array indexes configurable, it will be a source of bugs and confusion. We cannot force users to pass transforms when instantiating an expression parser, but we can make it every explicit there and spend effort in explaining this correctly in docs and examples.
So, I will try to keep the existing behavior with math
, transform
, and mathWithTransform
.
I'm making good progress. The bundles are mostly working again. Still struggling with supporting both a mathjs instance where functions are coupled and listening for changes in config, and at the same time offering standalone, pure functions.
There was an internal function distribution
which needed to be refactored. It's now turned into three standalone functions random
, randomInt
, and pickRandom
. I love how the refactoring into typed-function worked out, see https://github.com/josdejong/mathjs/commit/4d037c103ce2d1cf4a2d70045012c50cc6ee7a19 :sunglasses:.
The bundles and all testing on Travis is working again :tada:
I've now renamed the function and methods eval
to evaluate
. When using import { eval } from 'mathjs'
, it conflicts with the build in eval
function of JavaScript. We could live with that (except on older browsers), but I think it is important to keep it strictly separate to prevent errors or confusion here.
I agree, could we leave eval
as a method on the mathjs
namespace in addition (plus possibly depreciate)?
:+1: Right now I've kept the old eval
function and methods working, and they log a deprecation message once, see for example eval.js.
Perfect 👍
I've implemented snapshot testing to validate that both the instances, expression parser, and the ES6 exports contain all functions and constants they should have. Fixed the things that where still missing in the ES6 export :).
EDIT: so this basically means that the refactored code has everything working that the previous version had. Next steps is to get the tree-shaking working property, and work out number-only implementations of all relevant functions.
Fixed tree-shaking not working, see 97c16e780b24910a5f214fdb650f25deb4cc7990 :tada: , starting to get the hang of it.
It's unfortunate but because of the factory functions you have to make clear to Webpack that these function are pure using this /* #__PURE__ */
annotation.
I've flattened everything in the mathjs instances, and created fallbacks for compatibility. So for example math.type.Complex
is moved to math.Complex
, and math.expression.node.SymbolNode
is moved to math.SymbolNode
.
Reason for this is that we need to keep the API's consistent and simple, and the ES6 api only allows flat imports (i.e. import { SymbolNode } from 'mathjs'
. Having some classes and functions nested made things really complicated.
@josdejong if you want to keep the old API:
Import { expression } from ‘mathjs’
Import { SymbolNode } from ‘mathjs/expression’
Import { Complex } from ‘mathjs/type’
Though perhaps flat imports is simpler.
Really looking forward to modular mathjs!
@ChristopherChudzicki Yes that's true, I've been thinking about that option as well. What makes it complicated is that we also want to offer different versions of the API: the full version, or implementations for just numbers or BigNumbers, and somehow want to expose the factory functions. I was thinking about something like this:
import { add, multiply } from 'mathjs'
import { add, multiply } from 'mathjs/number' // functions only supporting numbers
import { add, multiply } from 'mathjs/bignumber' // functions only supporting BigNumbers
import { createAdd, createMultiply } from 'mathjs/factory' // factory functions to do some cooking yourself
But if we also would have things like Import { SymbolNode } from 'mathjs/expression/node'
it becomes too complicated I think. At least it drove my crazy already during the refactor, now that I've flattened it things become way simpler :D
@josdejong Ah, that would be overwhelming. Makes sense!
I've worked out the use cases in more detail, and tried to come up with a simple API to support all the different things we want (which is quite a few!). I've written down a summary of the use cases and an explanation of the ideas behind the API here:
https://github.com/josdejong/mathjs/blob/modular_architecture/test-tree-shaking/README.md
Amongst others, I introduce the concept of a "recipe" and worked out a POC for this. I have no idea if this idea resonates with you guys, feedback is very welcome! I find it hard to come up with naming for the different concepts (factories, recipes, functions).
See also the folder test-tree-shaking
to try everything out for yourself.
!!! I really need your inputs !!!
@josdejong I read over your readme, and I like it! It looks like a good way to design things.
"recipe" is kinda an awkward term, but, like you said, naming things is hard. I don't have a better suggestion.
One thing I found confusing was the following snippet:
const { divide, sin, pi } = create({ addRecipe, divideRecipe })
Where are pi
and sin
coming from? create()
is only being asked for add
and divide
, and pi
and sin
seem to come out of nowhere.
This would