Refactor request: Use colors/safe

[kriskowal]

I am working with a project that would benefit from running in an environment where the shared primordial objects like String.prototype are frozen to prevent supply-chain attacks. That project uses jasmine and this jasmine-spec-runner.

In many cases, replacing import "colors" with import colors from "colors" then changing terms like "failed".red to colors.red("failed") is sufficient. However, this project makes extensive use of themes to patch String.prototype on-the-fly.

I would like to propose a refactor that captures the configured styles on a theme module and uses the theme module instead of colors/safe. Would that change be welcome?

[bcaudan]

Hi @kriskowal,

This is a legitimate request, it will probably be a breaking change though.

From my understanding, DisplayProcessor would then be dependent on this Theme module:

export class DisplayProcessor {
    protected configuration: Configuration;
    protected theme: Theme;

    constructor(configuration: Configuration, theme: Theme) {
        this.configuration = configuration;
        this.theme = theme;
    }

    ...
}

export class SpecColorsProcessor extends DisplayProcessor {
    public displaySuccessfulSpec(spec: CustomReporterResult, log: string): string {
        return this.theme.successful(log);
    }

    ...
}

Does it look like what you had in mind?

[kriskowal]

Any design that does not mutate String.prototype is sufficient for the scope of this request. You can test that assertion by freezing String.prototype before running tests. Or, to freeze everything, you can import "ses" and call lockdown(), to be sure.

On an unrelated note, any design that eliminates shared mutable state in the scope of modules generally composes better and is easier to test. I would be delighted if the API had a workflow like:

1. create a theme from configuration
2. create a reporter with the theme
3. run tests with a reporter

I am not intimately familiar with jasmine-spec-reporter internals, but I think that what you have outlined is what I have described!

[bcaudan]

Available in jasmine-spec-reporter@6.0.0

[kriskowal]

Thank you for this. I can confirm that this works as intended!