Replace default exports with individual exports and namespaced imports

[becca-bailey]

Closes #650
Closes #1909

Updates default export objects to individual exports to better support webpack tree-shaking. In order to preserve the namespacing (e.g. import { Helpers } from 'victory-core'), this adds the babel export namespace proposal in order to support syntax like export * as Helpers from 'victory-util/helpers', though it sounds like we might need to upgrade to webpack v5 to support tree-shaking with this syntax.

I am currently running into this issue with any helper methods that are mocked using sinon.stub(...) in the tests. I will continue to investigate this!

This also includes some testing improvements, including a TEST_MODULE env variable to isolate a single module. For example, now you can run:

TEST_MODULE=victory-axis nps test

This also adds test watcher scripts so you can run nps test.watch or nps karma.watch.

[becca-bailey]

I'm still working on TextSize - this one broke some tests and I'm still determining whether this is an issue with sinon mocking or some other kind of bug. Storybook looks the same, though.

[ryan-roemer]

@beccanelson If you're still spinning your wheels on this, feel free to just give me some reproduction instructions and I can take a look too!

[becca-bailey]

I figured it out - the earlier version of sinon we were using for test stubbing doesn't support module exports. Upgrading sinon seems to be doing the trick to fix the tests.

[boygirl]

@beccanelson This is all looking great to me! Please remove the draft status when you're ready so the chromatic tests aren't skipped

[ryan-roemer]

LGTM!

[becca-bailey]

I just pushed up the sinon update, which should fix some of the test mocking issues I was having. Do we also want to include all the helper-methods.js files in each chart package, or should we save that for another PR?

[becca-bailey]

Hmm, maybe there's something else going on here. I still see some of the test failures related to the TextSize module.

[boygirl]

@beccanelson the tests I'm seeing fail look like they're related to fixLabelOverlap on VictoryAxis

[becca-bailey]

@boygirl Yes, from what I can figure out we're trying to stub the result of TextSize.approximateTextSize to always return a consistent value, but that stub isn't working so it's using the default calculated size.

[boygirl]

ah, gotcha

[ryan-roemer]

If there is an error in the test before this line of code your stub.restore() call in line 23 with also fail. Better to switch back to the sandbox approach because that is guaranteed to be instantiated in beforeEach

[becca-bailey]

@boygirl Do you have thoughts about this stubbable implementation that @ryan-roemer added? Normally I'm not a huge fan of doing hacky stuff like this for the sake of testing, but I also don't want to lose the benefit of the existing tests or spend a ton of time trying to figure out why this particular module isn't stubbable.

The other option we have (which probably isn't much better) is to do some old-fashioned dependency injection and accept the text size as an optional prop.

[boygirl]

@beccanelson I agree that the stubbable implementation isn't great, but I think it's the best option for the moment. Let's go with that so you don't have to keep spinning your wheels on this.

[ryan-roemer]

I totally agree with y’all. 😛 If it was more than just one, we’d probably want to look for more first class solutions.

[becca-bailey]

I rebased on main, and got some test errors that can be traced back to f873926. It seems like the latest sinon version and its dependencies are not playing well with webpack 5. 😭

[ryan-roemer]

@beccanelson I don't have time for this today, but I can dig into this tomorrow if you'd like?

[becca-bailey]

@ryan-roemer No rush on this, but I would appreciate some help whenever you're free!

[ryan-roemer]

I'm up to date on this branch and getting the same error as CI, namely:

export 'LabelHelpers'.'getTextPolarAnchor' (imported as 'LabelHelpers') was not found in 'victory-core' (possible exports: getDegrees, getPolarAngle, getPolarTextAnchor, getPolarVerticalAnchor, getProps, getText)

I think this may indicate potential issues with our upstream consumers for the exports change, so I'll dig in more tomorrow about implications. (tl;dr, we want our es/**/*.js output to be straight up buildable with webpack / other bundlers without needing further babel transformations, and the namespace export might not actually be...)

[ryan-roemer]

Here's what is currently failing:

/*!*************************************************************!*\
  !*** ../../node_modules/parse5/lib/parser/parser_stream.js ***!
  \*************************************************************/
/***/ ((module, __unused_webpack_exports, __webpack_require__) => {

"use strict";


var WritableStream = __webpack_require__(/*! stream */ "?2cec").Writable,
    inherits = __webpack_require__(/*! util */ "../../node_modules/util/util.js").inherits,
    Parser = __webpack_require__(/*! ./index */ "../../node_modules/parse5/lib/parser/index.js");

var ParserStream = module.exports = function (options) {
    WritableStream.call(this);

    this.parser = new Parser(options);

    this.lastChunkWritten = false;
    this.writeCallback = null;
    this.pausedByScript = false;

    this.document = this.parser.treeAdapter.createDocument();

    this.pendingHtmlInsertions = [];

    this._resume = this._resume.bind(this);
    this._documentWrite = this._documentWrite.bind(this);
    this._scriptHandler = this._scriptHandler.bind(this);

    this.parser._bootstrap(this.document, null);
};

// ==> THIS IS FAILING <===
inherits(ParserStream, WritableStream);

//WritableStream implementation
ParserStream.prototype._write = function (chunk, encoding, callback) {
    this.writeCallback = callback;
    this.parser.tokenizer.write(chunk.toString('utf8'), this.lastChunkWritten);
    this._runParsingLoop();
};

ParserStream.prototype.end = function (chunk, encoding, callback) {
    this.lastChunkWritten = true;
    WritableStream.prototype.end.call(this, chunk || '', encoding, callback);
};

//Scriptable parser implementation
ParserStream.prototype._runParsingLoop = function () {
    this.parser.runParsingLoopForCurrentChunk(this.writeCallback, this._scriptHandler);
};

[becca-bailey]

@ryan-roemer @boygirl I added a node polyfill to the test config that seems to fix the issue in parser.js, and fixed a bug (I think this was pre-existing) where the wrong function name was being called from LabelHelpers. Once everything is passing I should be able to get this merged in today!