Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support require('style9') #89

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

SukkaW
Copy link
Contributor

@SukkaW SukkaW commented Feb 5, 2023

Previously, style9 only supports import xxx from 'style9'.
The PR adds support for const xxx = require('style9').

The unit test case is added in __tests__/code/import.js.

@SukkaW
Copy link
Contributor Author

SukkaW commented Feb 5, 2023

Seems that CI failed due to test coverage. Let me see what I can do.


Update

The test coverage reaches 100% and the CI is passed.

@johanholmerin
Copy link
Owner

The mutation testing report(available at the action run page) shows that more tests are needed for negative situations, i.e. code that shouldn't be processed.

Examples of situations that may need to be tested:

const style9 = require(style9);
let style9 = require('style9');
var style9 = require('style9');
const { create } = require('style9');
const style9 = foo.require('style9');
const create = require('style9').create;
const style9 = globalThis.require('style9');
require('style9')();
require('style9').create();
foo(require('style9'));
const style9 = require('style9', foo);

@SukkaW
Copy link
Contributor Author

SukkaW commented Feb 6, 2023

const style9 = require(style9);
let style9 = require('style9');
var style9 = require('style9');
const { create } = require('style9');
const style9 = foo.require('style9');
const create = require('style9').create;
const style9 = globalThis.require('style9');
require('style9')();
require('style9').create();
foo(require('style9'));
const style9 = require('style9', foo);

Sure, I will add the cases.


Added in johanholmerin/style9@80e3f9c (#89)

@johanholmerin
Copy link
Owner

The mutation tests are still lacking some coverage. The cases above were only examples, they may not be exactly what is required. Check the test report and you should be able to see what's needed.

@johanholmerin
Copy link
Owner

@SukkaW Is this PR still active?

@SukkaW SukkaW marked this pull request as draft March 7, 2023 04:40
@SukkaW
Copy link
Contributor Author

SukkaW commented Mar 7, 2023

I have been a bit busy recently. I will draft the PR for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants