Pdcl 13739

[brenthosie]

Description

Turbine will overwhelm the console when a user's implementation (either directly or indirectly through an extension they installed) accesses deprecated features. This changes aims to:

1. audit the project's dependencies
2. remove security vulnerabilities
3. log deprecation to the console 1 time when outputEnabled = false

Related Issue

Motivation and Context

Closes #184

1. Remove vulnerable dependencies
2. Remove inject-loader and turn those test files into injectable factories
3. Remove dead code paths on production build
4. Ensure injectable modules have their required dependencies passed for production bundling
5. only console.warn once when a deprecated turbine method is accessed
6. update build & test process

How Has This Been Tested?

Screenshots (if appropriate):

Removing inject-loader

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

This is a major change that could have unforseen consequences. Must review carefully.

Checklist:

• I have signed the Adobe Open Source CLA.
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[brenthosie]

When removing inject-loader, here's the general pattern:

old module

var depA = require('./deps/a);
var depb = require('./deps/b);

module.exports = function theThing(runtimeDep) { ... }

New pattern

var validateInjectedParams = require('./helpers/validate-expected-inject-params');

// injectableThing allows us to remove inject-loader
function injectableThing({ depA, depB}) {
  return function theThing(runtimeDep) { ... }
}

var validateInjection = validateInjectedParams(injectableThing);

// since we test using the injectable factory, this ensures we export a fully-ready
// default export for production
module.exports = validateInjection({
  depA: require('./dep/a'),
  depB: require('./dep/b')
});

if (unit_test_mode) {
  // provided for test files
  module.exports.injectableThing = validateInjection;
}

[brenthosie]

This is used throughout Turbine now to control if we expose injectable functions or only export the default module with the real run-time dependencies.

[brenthosie]

rollup is our production bundler. enusure REACTOR_KARMA_CI_UNIT_TEST_MODE is gone.

[brenthosie]

This test file is loaded first in the unit test suite and its job is to ensure that when we call the default export of these injectable modules that they don't throw any errors from our new auto validation function that checks we've injected the right amount of params for production.

[brenthosie]

This was the original ask of the ticket pdcl-13979: Turbine was too chatty in a production environment because logger.deprecation was allowed to call the console even when setDebug(false) was called. This file now holds up to 100 unique log messages to de-duplicate and ignore. once the buffer fills up, the first entry is ejected to make room for a new call.

The goal here is if we see 5 unique deprecation messages over and over and over, we log each of them once and never again when outputEnabled = false.

[brenthosie]

The integration test allows us to really boot up Turbine and ensure it doesn't throw any errors while doing so.

[markhicken]

It looks like turbine is already pretty good about telling where deprecation notices came from. Is this ever called by 3rd party extensions?

[brenthosie]

Yes, it can be called by any 3rd party extension using turbine.logger

[markhicken]

I like the idea of having an in-browser integration test suite, but I'm not sure we should build it out beyond tests that provide more coverage than the existing unit tests.

It would be cool if it was smart enough to receive a real build that came from Forge. Maybe we can use a mock build (container) and also support providing a real build? Or perhaps they are 2 different test tasks? We could make it so integration tests could be called individually from a separate workflow.

[brenthosie]

I think we should at least come up with a few containers that hit major code paths. As an example, this will test the ...ToQueue files because action/condition sequencing is on but this container misses testing things like createRunAction which is synchronous.