Skip to content

Commit

Permalink
Merge branch 'version-4' into legacy-packages
Browse files Browse the repository at this point in the history
  • Loading branch information
benmccann authored Apr 15, 2023
2 parents 0068873 + 39333b1 commit 966079b
Show file tree
Hide file tree
Showing 30 changed files with 583 additions and 61 deletions.
25 changes: 22 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,17 @@ jobs:
timeout-minutes: 15
strategy:
matrix:
node-version: [14, 16, 18]
os: [ubuntu-latest, windows-latest, macOS-latest]
include:
- node-version: 14
os: ubuntu-latest
- node-version: 14
os: windows-latest
- node-version: 14
os: macOS-latest
- node-version: 16
os: ubuntu-latest
- node-version: 18
os: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
Expand All @@ -37,7 +46,17 @@ jobs:
timeout-minutes: 10
strategy:
matrix:
os: [ubuntu-latest, windows-latest, macOS-latest]
include:
- node-version: 14
os: ubuntu-latest
- node-version: 14
os: windows-latest
- node-version: 14
os: macOS-latest
- node-version: 16
os: ubuntu-latest
- node-version: 18
os: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
Expand Down
10 changes: 8 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,14 @@

## Unreleased (4.0)

* Minimum supported Node version is now Node 14
* Minimum supported webpack version is now webpack 5
* **breaking** Minimum supported Node version is now Node 14
* **breaking** Minimum supported webpack version is now webpack 5
* **breaking** Minimum supported TypeScript version is now TypeScript 5 (it will likely work with lower versions, but we make no guarantees about that)
* **breaking** Stricter types for `createEventDispatcher` (see PR for migration instructions) ([#7224](https://github.com/sveltejs/svelte/pull/7224))
* **breaking** Stricter types for `Action` and `ActionReturn` (see PR for migration instructions) ([#7224](https://github.com/sveltejs/svelte/pull/7224))
* Add `a11y no-noninteractive-element-interactions` rule ([#8391](https://github.com/sveltejs/svelte/pull/8391))
* Add `a11y-no-static-element-interactions`rule ([#8251](https://github.com/sveltejs/svelte/pull/8251))
* Bind `null` option and input values consistently ([#8312](https://github.com/sveltejs/svelte/issues/8312))

## Unreleased (3.0)

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
},
"types": "types/runtime/index.d.ts",
"scripts": {
"test": "npm run test:unit && npm run test:integration",
"test": "npm run test:unit && npm run test:integration && echo \"manually check that there are no type errors in test/types by opening the files in there\"",
"test:integration": "mocha --exit",
"test:unit": "mocha --config .mocharc.unit.js --exit",
"quicktest": "mocha --exit",
Expand Down
14 changes: 14 additions & 0 deletions site/content/docs/06-accessibility-warnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,20 @@ Some HTML elements have default ARIA roles. Giving these elements an ARIA role t

---

### `a11y-no-noninteractive-element-interactions`

A non-interactive element does not support event handlers (mouse and key handlers). Non-interactive elements include `<main>`, `<area>`, `<h1>` (,`<h2>`, etc), `<p>`, `<img>`, `<li>`, `<ul>` and `<ol>`. Non-interactive [WAI-ARIA roles](https://www.w3.org/TR/wai-aria-1.1/#usage_intro) include `article`, `banner`, `complementary`, `img`, `listitem`, `main`, `region` and `tooltip`.

```sv
<!-- `A11y: Non-interactive element <li> should not be assigned mouse or keyboard event listeners.` -->
<li on:click={() => {}} />
<!-- `A11y: Non-interactive element <div> should not be assigned mouse or keyboard event listeners.` -->
<div role="listitem" on:click={() => {}} />
```

---

### `a11y-no-noninteractive-element-to-interactive-role`

[WAI-ARIA](https://www.w3.org/TR/wai-aria-1.1/#usage_intro) roles should not be used to convert a non-interactive element to an interactive element. Interactive ARIA roles include `button`, `link`, `checkbox`, `menuitem`, `menuitemcheckbox`, `menuitemradio`, `option`, `radio`, `searchbox`, `switch` and `textbox`.
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/compile/compiler_warnings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,18 @@ export default {
code: 'a11y-no-redundant-roles',
message: `A11y: Redundant role '${role}'`
}),
a11y_no_static_element_interactions: (element: string, handlers: string[]) => ({
code: 'a11y-no-static-element-interactions',
message: `A11y: <${element}> with ${handlers.join(', ')} ${handlers.length === 1 ? 'handler' : 'handlers'} must have an ARIA role`
}),
a11y_no_interactive_element_to_noninteractive_role: (role: string | boolean, element: string) => ({
code: 'a11y-no-interactive-element-to-noninteractive-role',
message: `A11y: <${element}> cannot have role '${role}'`
}),
a11y_no_noninteractive_element_interactions: (element: string) => ({
code: 'a11y-no-noninteractive-element-interactions',
message: `A11y: Non-interactive element <${element}> should not be assigned mouse or keyboard event listeners.`
}),
a11y_no_noninteractive_element_to_interactive_role: (role: string | boolean, element: string) => ({
code: 'a11y-no-noninteractive-element-to-interactive-role',
message: `A11y: Non-interactive element <${element}> cannot have interactive role '${role}'`
Expand Down
58 changes: 54 additions & 4 deletions src/compiler/compile/nodes/Element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import StyleDirective from './StyleDirective';
import Text from './Text';
import { namespaces } from '../../utils/namespaces';
import map_children from './shared/map_children';
import { is_name_contenteditable, get_contenteditable_attr } from '../utils/contenteditable';
import { is_name_contenteditable, get_contenteditable_attr, has_contenteditable_attr } from '../utils/contenteditable';
import { regex_dimensions, regex_starts_with_newline, regex_non_whitespace_character, regex_box_size } from '../../utils/patterns';
import fuzzymatch from '../../utils/fuzzymatch';
import list from '../../utils/list';
Expand Down Expand Up @@ -102,6 +102,15 @@ const a11y_interactive_handlers = new Set([
'mouseup'
]);

const a11y_recommended_interactive_handlers = new Set([
'click',
'mousedown',
'mouseup',
'keypress',
'keydown',
'keyup'
]);

const a11y_nested_implicit_semantics = new Map([
['header', 'banner'],
['footer', 'contentinfo']
Expand Down Expand Up @@ -738,17 +747,19 @@ export default class Element extends Node {
}
}

const role = attribute_map.get('role');
const role_static_value = role?.get_static_value() as ARIARoleDefinitionKey;
const role_value = (role ? role_static_value : get_implicit_role(this.name, attribute_map)) as ARIARoleDefinitionKey;

// no-noninteractive-tabindex
if (!this.is_dynamic_element && !is_interactive_element(this.name, attribute_map) && !is_interactive_roles(attribute_map.get('role')?.get_static_value() as ARIARoleDefinitionKey)) {
if (!this.is_dynamic_element && !is_interactive_element(this.name, attribute_map) && !is_interactive_roles(role_static_value)) {
const tab_index = attribute_map.get('tabindex');
if (tab_index && (!tab_index.is_static || Number(tab_index.get_static_value()) >= 0)) {
component.warn(this, compiler_warnings.a11y_no_noninteractive_tabindex);
}
}

// role-supports-aria-props
const role = attribute_map.get('role');
const role_value = (role ? role.get_static_value() : get_implicit_role(this.name, attribute_map)) as ARIARoleDefinitionKey;
if (typeof role_value === 'string' && roles.has(role_value)) {
const { props } = roles.get(role_value);
const invalid_aria_props = new Set(aria.keys().filter(attribute => !(attribute in props)));
Expand All @@ -762,6 +773,45 @@ export default class Element extends Node {
}
});
}

// no-noninteractive-element-interactions
if (
!has_contenteditable_attr(this) &&
!is_hidden_from_screen_reader(this.name, attribute_map) &&
!is_presentation_role(role_static_value) &&
((!is_interactive_element(this.name, attribute_map) &&
is_non_interactive_roles(role_static_value)) ||
(is_non_interactive_element(this.name, attribute_map) && !role))
) {
const has_interactive_handlers = handlers.some((handler) => a11y_recommended_interactive_handlers.has(handler.name));
if (has_interactive_handlers) {
component.warn(this, compiler_warnings.a11y_no_noninteractive_element_interactions(this.name));
}
}

const has_dynamic_role = attribute_map.get('role') && !attribute_map.get('role').is_static;

// no-static-element-interactions
if (
!has_dynamic_role &&
!is_hidden_from_screen_reader(this.name, attribute_map) &&
!is_presentation_role(role_static_value) &&
!is_interactive_element(this.name, attribute_map) &&
!is_interactive_roles(role_static_value) &&
!is_non_interactive_element(this.name, attribute_map) &&
!is_non_interactive_roles(role_static_value) &&
!is_abstract_role(role_static_value)
) {
const interactive_handlers = handlers
.map((handler) => handler.name)
.filter((handlerName) => a11y_interactive_handlers.has(handlerName));
if (interactive_handlers.length > 0) {
component.warn(
this,
compiler_warnings.a11y_no_static_element_interactions(this.name, interactive_handlers)
);
}
}
}

validate_special_cases() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ export default class AttributeWrapper extends BaseAttributeWrapper {
}

if (is_indirectly_bound_value) {
const update_value = b`${element.var}.value = ${element.var}.__value;`;
const update_value = b`@set_input_value(${element.var}, ${element.var}.__value);`;
block.chunks.hydrate.push(update_value);

updater = b`
Expand Down
9 changes: 7 additions & 2 deletions src/compiler/compile/utils/a11y.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ const non_interactive_roles = new Set(
// 'toolbar' does not descend from widget, but it does support
// aria-activedescendant, thus in practice we treat it as a widget.
// focusable tabpanel elements are recommended if any panels in a set contain content where the first element in the panel is not focusable.
!['toolbar', 'tabpanel'].includes(name) &&
// 'generic' is meant to have no semantic meaning.
!['toolbar', 'tabpanel', 'generic'].includes(name) &&
!role.superClass.some((classes) => classes.includes('widget'))
);
})
Expand All @@ -31,7 +32,11 @@ const non_interactive_roles = new Set(
);

const interactive_roles = new Set(
non_abstract_roles.filter((name) => !non_interactive_roles.has(name))
non_abstract_roles.filter((name) =>
!non_interactive_roles.has(name) &&
// 'generic' is meant to have no semantic meaning.
name !== 'generic'
)
);

export function is_non_interactive_roles(role: ARIARoleDefinitionKey) {
Expand Down
19 changes: 13 additions & 6 deletions src/runtime/action/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/**
* Actions can return an object containing the two properties defined in this interface. Both are optional.
* - update: An action can have a parameter. This method will be called whenever that parameter changes,
* immediately after Svelte has applied updates to the markup.
* immediately after Svelte has applied updates to the markup. `ActionReturn` and `ActionReturn<never>` both
* mean that the action accepts no parameters, which makes it illegal to set the `update` method.
* - destroy: Method that is called after the element is unmounted
*
* Additionally, you can specify which additional attributes and events the action enables on the applied element.
Expand All @@ -25,8 +26,8 @@
*
* Docs: https://svelte.dev/docs#template-syntax-element-directives-use-action
*/
export interface ActionReturn<Parameter = any, Attributes extends Record<string, any> = Record<never, any>> {
update?: (parameter: Parameter) => void;
export interface ActionReturn<Parameter = never, Attributes extends Record<string, any> = Record<never, any>> {
update?: [Parameter] extends [never] ? never : (parameter: Parameter) => void;
destroy?: () => void;
/**
* ### DO NOT USE THIS
Expand All @@ -42,15 +43,21 @@ export interface ActionReturn<Parameter = any, Attributes extends Record<string,
* The following example defines an action that only works on `<div>` elements
* and optionally accepts a parameter which it has a default value for:
* ```ts
* export const myAction: Action<HTMLDivElement, { someProperty: boolean }> = (node, param = { someProperty: true }) => {
* export const myAction: Action<HTMLDivElement, { someProperty: boolean } | undefined> = (node, param = { someProperty: true }) => {
* // ...
* }
* ```
* `Action<HTMLDivElement>` and `Action<HTMLDiveElement, never>` both signal that the action accepts no parameters.
*
* You can return an object with methods `update` and `destroy` from the function and type which additional attributes and events it has.
* See interface `ActionReturn` for more details.
*
* Docs: https://svelte.dev/docs#template-syntax-element-directives-use-action
*/
export interface Action<Element = HTMLElement, Parameter = any, Attributes extends Record<string, any> = Record<never, any>> {
<Node extends Element>(node: Node, parameter?: Parameter): void | ActionReturn<Parameter, Attributes>;
export interface Action<Element = HTMLElement, Parameter = never, Attributes extends Record<string, any> = Record<never, any>> {
<Node extends Element>(...args: [Parameter] extends [never] ? [node: Node] : undefined extends Parameter ? [node: Node, parameter?: Parameter] : [node: Node, parameter: Parameter]): void | ActionReturn<Parameter, Attributes>;
}

// Implementation notes:
// - undefined extends X instead of X extends undefined makes this work better with both strict and nonstrict mode
// - [X] extends [never] is needed, X extends never would reduce the whole resulting type to never and not to one of the condition outcomes
34 changes: 24 additions & 10 deletions src/runtime/internal/lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,17 @@ export function onDestroy(fn: () => any) {
get_current_component().$$.on_destroy.push(fn);
}

export interface EventDispatcher<EventMap extends Record<string, any>> {
// Implementation notes:
// - undefined extends X instead of X extends undefined makes this work better with both strict and nonstrict mode
// - [X] extends [never] is needed, X extends never would reduce the whole resulting type to never and not to one of the condition outcomes
<Type extends keyof EventMap>(
...args: [EventMap[Type]] extends [never] ? [type: Type, parameter?: null | undefined, options?: DispatchOptions] :
null extends EventMap[Type] ? [type: Type, parameter?: EventMap[Type], options?: DispatchOptions] :
undefined extends EventMap[Type] ? [type: Type, parameter?: EventMap[Type], options?: DispatchOptions] :
[type: Type, parameter: EventMap[Type], options?: DispatchOptions]): boolean;
}

export interface DispatchOptions {
cancelable?: boolean;
}
Expand All @@ -68,20 +79,23 @@ export interface DispatchOptions {
* [CustomEvent](https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent).
* These events do not [bubble](https://developer.mozilla.org/en-US/docs/Learn/JavaScript/Building_blocks/Events#Event_bubbling_and_capture).
* The `detail` argument corresponds to the [CustomEvent.detail](https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent/detail)
* property and can contain any type of data.
* property and can contain any type of data.
*
* The event dispatcher can be typed to narrow the allowed event names and the type of the `detail` argument:
* ```ts
* const dispatch = createEventDispatcher<{
* loaded: never; // does not take a detail argument
* change: string; // takes a detail argument of type string, which is required
* optional: number | null; // takes an optional detail argument of type number
* }>();
* ```
*
* https://svelte.dev/docs#run-time-svelte-createeventdispatcher
*/
export function createEventDispatcher<EventMap extends {} = any>(): <
EventKey extends Extract<keyof EventMap, string>
>(
type: EventKey,
detail?: EventMap[EventKey],
options?: DispatchOptions
) => boolean {
export function createEventDispatcher<EventMap extends Record<string, any> = any>(): EventDispatcher<EventMap> {
const component = get_current_component();

return (type: string, detail?: any, { cancelable = false } = {}): boolean => {
return ((type: string, detail?: any, { cancelable = false } = {}): boolean => {
const callbacks = component.$$.callbacks[type];

if (callbacks) {
Expand All @@ -95,7 +109,7 @@ export function createEventDispatcher<EventMap extends {} = any>(): <
}

return true;
};
}) as EventDispatcher<EventMap>;
}

/**
Expand Down
7 changes: 4 additions & 3 deletions test/js/samples/select-dynamic-value/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {
insert,
noop,
safe_not_equal,
select_option
select_option,
set_input_value
} from "svelte/internal";

function create_fragment(ctx) {
Expand All @@ -24,9 +25,9 @@ function create_fragment(ctx) {
option1 = element("option");
option1.textContent = "2";
option0.__value = "1";
option0.value = option0.__value;
set_input_value(option0, option0.__value);
option1.__value = "2";
option1.value = option1.__value;
set_input_value(option1, option1.__value);
},
m(target, anchor) {
insert(target, select, anchor);
Expand Down
3 changes: 0 additions & 3 deletions test/runtime-puppeteer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,6 @@ describe('runtime (puppeteer)', () => {

function runTest(dir, hydrate, is_first_run) {
if (dir[0] === '.') return;
// MEMO: puppeteer can not execute Chromium properly with Node8,10 on Linux at GitHub actions.
const { version } = process;
if ((version.startsWith('v8.') || version.startsWith('v10.')) && process.platform === 'linux') return;

const config = loadConfig(`${__dirname}/samples/${dir}/_config.js`);
const solo = config.solo || /\.solo/.test(dir);
Expand Down
28 changes: 28 additions & 0 deletions test/runtime/samples/binding-select-null-placeholder/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
const items = [ { id: 'a' }, { id: 'b' } ];

export default {
props: {
foo: null,
items
},

test({ assert, component, target }) {
const select = target.querySelector( 'select' );
const options = target.querySelectorAll( 'option' );

assert.equal( options[0].selected, true );
assert.equal( options[0].disabled, true );
assert.equal( options[1].selected, false );
assert.equal( options[1].disabled, false );

// placeholder option value must be blank string for native required field validation
assert.equal( options[0].value, '' );
assert.equal( select.checkValidity(), false );

component.foo = items[0];

assert.equal( options[0].selected, false );
assert.equal( options[1].selected, true );
assert.equal( select.checkValidity(), true );
}
};
Loading

0 comments on commit 966079b

Please sign in to comment.