diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 000000000..f1437a618 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,133 @@ +Guide to Contributing +===================== + +Thanks for contributing to scala-js-dom! +We primarily accept PRs for: +* Adding facades for APIs documented in the spec +* Enhancing/fixing existing facades to match the spec +* Adding non-facade Scala utilities to complement specific facades +* Any other bug fixes etc. + +If you would like to make PR that doesn't fall under those categories, +please raise an issue first for discussion so we can give you the go-ahead! + +We look forward to your PRs! + +Contents: + +* Packages +* Files +* Facades +* Non-Facades +* Binary Compatibility +* Submitting a PR + + +Packages +======== + +In v1.x, there used to be sub-packages grouping some parts of the DOM API by major feature. +In v2.x, we've decided to put everything in `org.scalajs.dom` and get rid of sub-packages. + +The reason for this change is that the real DOM API isn't namespaced in anyway, and the decision +whether to group in a package or not, was subjective and inconsistent. + + +Files +===== + +* Use `package.scala` for a package object and nothing else. + Also don't include traits/classes/objects. + +* Match the filename to the trait/class/object in it; don't add multiple top-level types. + This is effectively Java-style. + + +Facades +======= + +We accept facades for any non-deprecated API documented in the spec, including experimental APIs or APIs not supported on all browsers. +* MDN: https://developer.mozilla.org/en-US/docs/Web/API +* WHATWG: https://spec.whatwg.org/ + +Please: +* Use `def` for read-only properties unless there is a compelling reason it should be a `val` + (i.e., the spec definitively states it is constant) +* Use `Double` for integer-values that can fall outside the range of `Int` +* Add scaladocs via copy-paste from MDN + + +Non-Facades +=========== + +* Implicit conversions should go in companion objects so that they are always in scope without the + need for imports. There's no need to group by feature, the types already specify the feature. + +* Add Scala-only utilities that pertain to a specific facade, in the facades companion object + Eg: helper constructors, legal facade values. + +* We currently don't see the need for Scala-only utilities that don't pertain to a specific facade, + or shouldn't be universally available (subjective judgement here). + If you believe you've got a compelling use case please raise an issue first to discuss. + +Binary Compatibility +==================== + +Binary compatibility for Scala.js facades is different than standard Scala. +The following are changes that are indeed incompatible in both formats: + +Don't: + * Remove a trait / class / object + * Change a class into a trait or vice versa + +Here is a non-exhaustive list of changes that would be binary-incompatible for Scala classes, but +are compatible for JS facade types: + +You can: + * Remove a member + * Change the type or signature of a member + * Add a field in a trait + * Add an abstract member in a trait + +To help us enforce binary compatibility, we use API reports. +They are auto-generated by running `prePR` in sbt and provide a concise summary of the entire API. +Note: We might automate binary compatibility checking in the future (see #503) but for now, +it's just a helpful tool for reviewing PRs. + +Here is an example of a binary _compatible_ change in #491 as it appears in the API report diff. +Note that `[JC]` stands for **J**avascript **C**lass, indicating that `HTMLAudioElement` is a facade type and thus this is a compatible change. +```diff +-raw/HTMLAudioElement[JC] def play(): Unit ++raw/HTMLAudioElement[JC] def play(): js.UndefOr[js.Promise[Unit]] +``` + +Here is an example of a binary _incompatible_ change in #458 as it appears in the API report diff. +Even though the `Fetch` object is a facade (a **J**avascript **O**bject), moving it out of the `experimental` package is not binary compatible. +(In this particular case, the change was accepted along with many binary-breaking changes going into 2.0.0.) +```diff +-experimental/Fetch[JO] def fetch(info: RequestInfo, init: RequestInit = null): js.Promise[Response] ++Fetch[JO] def fetch(info: RequestInfo, init: RequestInit = null): js.Promise[Response] +``` + +Anything annotated `[SC]`, `[ST]`, or `[SO]` in an API report is an ordinary Scala class/trait/object, +for which standard binary compatibility rules apply. + +If the above doesn't make sense to you, don't worry! +The majority of useful changes to scala-js-dom are indeed binary compatible. + + +Submitting a PR +=============== + +Once you're done making your changes... + +1. Run `sbt prePR` + +2. Run `git diff api-reports` and ensure that you aren't breaking backwards-binary-compatibility + (see above). + +3. Check in and commit the changes to `api-reports` + +4. Submit your PR + +5. Know that your contribution is appreciated, thank you!