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

refactor(interface): Use default export for implementation #141

Conversation

ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented Oct 22, 2019

⚠️ WARNING: BREAKING CHANGE

Fixes #59


This changes the implementation class syntax to allow CommonJS and ES2015 style default exports:

// CommonJS
/** @type {WeakMap<Foo, PrivateData>} */
const fooPrivateData = new WeakMap();

class Foo {
	// implementation
}
module.exports = exports = Foo;

export.init = function(instance, privateData) {
	fooPrivateData.set(instance, privateData);
}
// ES2015
/** @type {WeakMap<Foo, PrivateData>} */
const fooPrivateData = new WeakMap();

export default class Foo {
	// implementation
}

export function init(instance, privateData) {
	fooPrivateData.set(instance, privateData);
}

TODO:

  • Add tests for the importStar(…) helper.
  • Update documentation

add(x, y) {
return x + y;
}
};
```

> Note: It's also possible to use ES2015 module default export syntax:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not true; jsdom only operates on CommonJS modules.

Copy link
Contributor Author

@ExE-Boss ExE-Boss Oct 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, you do need to transpile ES2015 modules to be able to use them in Node.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. That is not a supported scenario, so please remove it.

@@ -74,6 +74,52 @@ const namedSetNew = Symbol("named property set new");
const namedSetExisting = Symbol("named property set existing");
const namedDelete = Symbol("named property delete");

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not interested in this addition. Please remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which? The JSDoc or the entire utils.importStar(…) function?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The importStar function itself.

@ExE-Boss ExE-Boss force-pushed the refactor/interface-implementation/use-default-export branch from d239363 to 3dc3ff3 Compare November 6, 2019 04:53
@domenic domenic closed this Jan 3, 2020
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.

Can implementation classes default-export implementation, instead of having a single export?
2 participants