Convert to ES Modules

[niksy]

As per #245.

This is inital conversion effort.

Couple of notes:

• Certain tests had to be modified to support strict mode.
• There are couple of tests that are failing so those should be updated.
• Currently tests are running only in Node, so browser testing should be added (maybe via Karma?).
• There’s still no process of creating CommonJS files from ESM files, but it shouldn’t be too hard to add that.
• Named exports should be added where appropriate.
• Add module as side-effect free (sideEffects: false in package.json)
• As final touch, little housekeeping could be added with ESLint, Prettier, Git commit hooks, …
• Automatic ES5 to ES6 convert via Lebab?
• What should be default consuming format, ESM or CJS?
  • If it’s ESM, CJS is available through const math = require('mout/cjs/math);
  • If it’s CJS, ESM is available through import math from 'mout/esm/math;

/cc @roboshoes @millermedeiros

[roboshoes]

Hey there,

first off thanks for the interest and time put into the upkeep of this. I agree that it's time to port this over maybe in an effort to hit v2.

At this point I always just want to surface the idea of converting it to typescript or at least offering type definition. At this point, not having types might be a bottle neck for some projects as it has been a several year long trend of going more towards type safe applications.

This would also allow us to use the typescript compiler to take some of the load off as far as conversion to different modes goes.

Thoughts?

PS: I'm looking into the source now and start a review soon.

[niksy]

I don't have that much experience with TypeScript, unfortunately :(, but I agree that this could definitely be combined effort with transitioning to ESM. Everything I've done here is based on my knowledge and experience of current standardized JS module format, sans TypeScript. It would be great if someone more versed in TS could chime in, maybe start with basic implementation and let others to do other work? That way we could all gain some experience.

[roboshoes]

I'm very familiar with it, so I can do the TS part. The change you are proposing is very inline with it anyway. I think I can take over after your commit and add the typings.

[niksy]

#264 (comment)

I’ve applied Prettier via ESLint and all the built files will be automatically fixed.

[niksy]

@roboshoes Prettier applied to all files. I haven’t used any ESLint rules yet, maybe we could use recommended set of rules for now?

[roboshoes]

Yeah we can use the recommended rule set and then adjust rules as needed. I can probably also generate a config based on the current code layout.

I will, however, move it into a separate config file. I don't think we should load up the package.json with config that doesn't need to be delivered to the client.

[roboshoes]

@niksy Are you ok with me adding a few commits to your branch PR. Might be the easiest way to work on this together.

[niksy]

Absolutely, go for it!

[roboshoes]

I have started to add typing. I can't currently add to this PR per say, because you would have to add me as a contributor to your forked repo.

I've started however with what this looks like: https://github.com/mout/mout/compare/niksy-esm-convert?expand=1

Look for some files like src/number/pad.ts so see the typing.

[roboshoes]

Well, there are two other ways we can do this both have essentially the same downside. We could write the tests also in typescript and depend on the actual typescript version of mout, then one could compile everything and run tests or we leave the same paths in the tests and compile the ja file side by side to the ts file.

They both have that one flaw that the tests do not run against the final package. That means you could have valid tests but a faulty package. Ultimately we have to generate the is code and move it into the root directory before npm publish. The nice thing about how the tests are right now, they require the Filder structure to be the way it has to be before publishing.

I have published a patch mout before where it didn't generate the cjs packages but the tests were still OK. Causing lots of builds to break.

This would be stopped this way.

That's the one argument for this. But like you said, it makes it a bit more complicated.

[niksy]

Hm, what about running tests through Rollup which will apply all necessary tooling on all the files - plugins for TypeScript, Babel, etc.? That way we’ll be running tests on compiled codebase which is essentially the same as the one which will be published as npm package.

The way I see it, we currently need TypeScript and Babel for compiling. Both are controlled with their respective configuration files which will be consumed with whatever tool we choose (babel-core, tsc, Rollup plugin, …).

Am I missing something here?

---

As for generating JS files option, I’m more inclined to the option of generating .js files along with source .ts files since it’s easier to see which package test refers to. Yes, it’s only one path part difference (/src/), but looking at the codebase maybe it won’t be understandable to newcomers. After all, testing and contributing should be easy. Maybe we should make the effort to leave testing to be easy as possible?

[roboshoes]

Well, currently the tests run against the compiled code. That's the point. It actually isn't using Babel right now just the typescript compiler.

No need for roll-up, since we don't want to tree shake any functions out. We could add babel maybe to transpile some features, but TS supports quite a bit.

We're always gonna have to compile before we test unless we write tests in TS. But I want to keep the tests to as close as possible to the published package. That's why I move the code to the top level. That means the tests run against the exact code that were publishing.

[niksy]

Seems to me that then we’re discussing the problem with accidentally not publishing code to npm, instead of the problem with testing the codebase.

So the question remains:

• Will we point tests to codebase which will eventually be published to npm?
  • That codebase is already ready for publishing to npm, which is one step less, but publishing to npm is IMO completely different process than testin
  • Drawback is that tests in repository point to completely different codebase which is non-existent in repository which can possibly break "intellisense" on GitHub when trying to reference original source code (e.g. with extensions like OctoLinker)
• Will we point tests to temporary codebase located at the the same location as source files?
  • Easy to reference where is source code
  • Cleanup process is needed, but so is for preparing codebase for npm

[niksy]

@roboshoes we took a little hiatus it seems :) What can we do next?

[roboshoes]

True. Sorry. I've had a lot of work the last couple weeks and didn't find much time. But that aside, I seem to also have missed your message from 13 days ago.

Your break down is absolutely correct. The problem I have with tests pointing at the actual code base is that it's not the code base being deployed. It makes for a cleaner dev setup, as you point out in point B, but it's less "safe".

Another thing to consider. The code base is now TS. So even if we keep the structure in a way where the the tests point at the source file, it begs to question if the tests should be in TS as well, also adding a compilation to the tests. Personally I wouldn't not vouch for that. I like to keep the tests as close to real use case as possible.

Ultimately the next big task before considering a deployment of v2 is resolving the remainding (~90 or so) complaints of the TS compiler. Most of it, is declaring types where it couldn't figure it out what it is, and fell back to any which should be avoided.

Before that can happen, we have to make a call towards the file structure. Personally I lean towards this. I feel strongly towards testing the code being shipped. However, given that I don't work much on MOUT in general, I'm open to move into another direction. However, we'd have to compile the code anyway to turn it into Javascript from Typescript before the tests can consume them.

[niksy]

True. Sorry. I've had a lot of work the last couple weeks and didn't find much time. But that aside, I seem to also have missed your message from 13 days ago.

No worries :)

Ultimately the next big task before considering a deployment of v2 is resolving the remainding (~90 or so) complaints of the TS compiler. Most of it, is declaring types where it couldn't figure it out what it is, and fell back to any which should be avoided.

I think I can try to help with that. As I said, I’m currently not that versed in writing TS, but I have it on my "to learn" list and maybe it’s time to try it out :)

Before that can happen, we have to make a call towards the file structure. Personally I lean towards this. I feel strongly towards testing the code being shipped. However, given that I don't work much on MOUT in general, I'm open to move into another direction. However, we'd have to compile the code anyway to turn it into Javascript from Typescript before the tests can consume them.

So, what about this:

• We have process which generates code for testing and publishing. The only difference is their location
• For testing, generated files are located at the same location as original TS files, deleted with test cleanup process
• For publishing, generated files are at the root of project so to avoid src/ structure, maybe deleted with postpublish process

What do you think about this? Step further would be to avoid src/ and keep everything in root.

[roboshoes]

Yeah that might be cleaner. I guess it would be similar to what it is right now. We just have to make sure that we bundle the publishing process with checking if the files exist in the root directory and then clean them up as well. It would still mean that we have different code bases for testing and deploying.

In your last point, are you suggesting simply moving the actual code base to the root. So to place the src where it lives after deployment? (This is how lodash does it)

I would consider that course.

[niksy]

Yeah that might be cleaner. I guess it would be similar to what it is right now. We just have to make sure that we bundle the publishing process with checking if the files exist in the root directory and then clean them up as well. It would still mean that we have different code bases for testing and deploying.

I think this shouldn’t cause any problem, especially with automation.

In your last point, are you suggesting simply moving the actual code base to the root. So to place the src where it lives after deployment? (This is how lodash does it)

I would consider that course.

Yes, moving source code to the root. But, looking at the file and folder structure right now maybe it would be best to keep it in the src/ folder. You are right, Lodash does that, but IMO it makes it harder to see what is meta and what is actual codebase. So maybe keep it in the src/ for now?

[roboshoes]

Yeah I agree. I like it in theory but it does feel like a lot of clutter. As you know, I think it's the right thing to write the package the way it gets deployed, but in our case, we have a lot of files and folder in the root directory then, so maybe we keep it in src and move it before deployment.

[niksy]

@roboshoes I’ve made some initial type changes, will probably start with more soon!

[roboshoes]

Awesome. I guess we also have to move the code generation back into source, as well as fix the test import path.

[roboshoes]

I'm continuously dropping the ball on this one. It's not forgotten. It's sitting in my inbox and I want to get to it.

[niksy]

@roboshoes ping 😄

[roboshoes]

Thanks for the ping actually. I couldn't really go outside today anyway (I live in Santa Cruz, so there are fires all around with horrible air quality)

I actually managed to update the code base in a way where we get no more TS errors. This means you can now go in and run npm run test and it will compile and run all tests successfully.

This does not mean that we actually have types for everything, but it does mean that there are no platent errors. This also means we can now generate .d.ts files for every file. Meaning when we deploy this package it will ship with its own type information. I think this adds quite some value as for lodash you have to install them desperately with npm install @types/lodash and this way the types are closely bundled to the source.

I guess the next step is actually updating the code base to utilize ES6 features more. I update some to use rest parameters already, while I made sure that the argument count is correct, but definitely not done.

[roboshoes]

I guess take a look @niksy and maybe you have time to update some of the source. Also make sure that npm test works for you. It generates the files and deletes them after the test. If it fails and didn't delete feel free to run node build.js prune as it does, well, prune the directory.

[roboshoes]

Alternatively I'm considering to merge this into a version-2 branch as this PR is getting very long.

[niksy]

Thanks for the ping actually. I couldn't really go outside today anyway (I live in Santa Cruz, so there are fires all around with horrible air quality)

Sorry to hear about that! Hope now everything is okay. 🤞

I’ve checked changes you made and it seems that everything is now OK! Your idea to merge this to new branch and closing this PR should make this more manageable, and then we can create PR for every other changes for new version.

After that, maybe I can try getting folder structure "in order" as mentioned in previous comment.

[roboshoes]

This has been merged into mout/v2.0.0. I have also deployed a alpha release to npm und mout@2.0.0-alpha.1

Preliminary testing seems positive.

[roboshoes]

Thank you so much for all this work. I just have to clean up the headers and then I should be good to go to deploy a to npm oficially