Skip to content

fix: add eslintrc files for check code convention #9104

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

fix: add eslintrc files for check code convention #9104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3825adb
fix: add eslintrc files for check naming convention
artxe Aug 14, 2023
5701e8a
fix: change camel-case declarations to snake-case
artxe Aug 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions packages/svelte/scripts/.eslintrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"rules": {
"id-match": [
"error",
"^\\$\\$$|^[_$]?[_$]?[a-z][a-z0-9]*(?:_[a-z]+)*(?:_\\d+)?$|^[A-Z][A-Z0-9]*(?:_[A-Z][A-Z0-9]*)*(?:_\\d+)?$|^(?:[A-Z][a-z0-9]*)+(?:_\\d+)?$",
{
"onlyDeclarations": true
}
]
}
}
19 changes: 12 additions & 7 deletions packages/svelte/scripts/compile-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
// Compile all Svelte files in a directory to JS and CSS files
// Usage: node scripts/compile-test.js <directory>

import { mkdirSync, readFileSync, writeFileSync } from 'fs';
import path from 'path';
import glob from 'tiny-glob/sync.js';
import {
mkdirSync as mkdir_sync,
Copy link
Member

@benmccann benmccann Aug 27, 2023

Choose a reason for hiding this comment

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

we don't have a convention of aliasing external packages to snake_case

Copy link
Contributor Author

@artxe artxe Aug 27, 2023

Choose a reason for hiding this comment

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

Yes, I understand.
If svelte 5 accepts the eslint rule,
You can use the eslint-disable-next-line id-match comment here.

Copy link
Member

@benmccann benmccann Aug 27, 2023

Choose a reason for hiding this comment

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

I appreciate that this rule found a lot of places that should be updated. However, I think it's just going to end up being too annoying to do eslint disable wherever we're calling an external package and think we probably can't accept this PR as a result. I really appreciate the effort you put into it. I think we could accept the fixes you identified if they were split out separately from the lint rule itself - though it might be better to wait until Svelte 5 for that as we're going to be throwing away all the existing compiler code

Copy link
Contributor Author

@artxe artxe Aug 28, 2023

Choose a reason for hiding this comment

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

Thank you for the thorough review.
The changes were intended for validating the effectiveness of the rule, and I believe applying them in the current version wouldn't bring significant benefits.

readFileSync as read_file_sync,
writeFileSync as write_file_sync
} from 'fs';

import { compile } from '../src/compiler/index.js';
import glob from 'tiny-glob/sync.js';
import path from 'path';

const cwd = path.resolve(process.argv[2]);

Expand All @@ -14,7 +19,7 @@ const options = [
['ssr', { generate: 'ssr' }]
];
for (const file of glob('**/*.svelte', { cwd })) {
const contents = readFileSync(`${cwd}/${file}`, 'utf-8').replace(/\r/g, '');
const contents = read_file_sync(`${cwd}/${file}`, 'utf-8').replace(/\r/g, '');
let w;
for (const [name, opts] of options) {
const dir = `${cwd}/_output/${name}`;
Expand All @@ -28,9 +33,9 @@ for (const file of glob('**/*.svelte', { cwd })) {
w = warnings;
}

mkdirSync(dir, { recursive: true });
js.code && writeFileSync(`${dir}/${file.replace(/\.svelte$/, '.js')}`, js.code);
css.code && writeFileSync(`${dir}/${file.replace(/\.svelte$/, '.css')}`, css.code);
mkdir_sync(dir, { recursive: true });
js.code && write_file_sync(`${dir}/${file.replace(/\.svelte$/, '.js')}`, js.code);
css.code && write_file_sync(`${dir}/${file.replace(/\.svelte$/, '.css')}`, css.code);
}

if (w) {
Expand Down
5 changes: 3 additions & 2 deletions packages/svelte/scripts/generate-dts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as fs from 'fs';
import { createBundle } from 'dts-buddy';

import { createBundle as create_bundle } from 'dts-buddy';

// It may look weird, but the imports MUST be ending with index.js to be properly resolved in all TS modes
for (const name of ['action', 'animate', 'easing', 'motion', 'store', 'transition']) {
Expand All @@ -14,7 +15,7 @@ fs.mkdirSync('./types/compiler', { recursive: true });
fs.writeFileSync('./types/compiler/preprocess.d.ts', `import '../index.js';`);
fs.writeFileSync('./types/compiler/interfaces.d.ts', `import '../index.js';`);

await createBundle({
await create_bundle({
output: 'types/index.d.ts',
compilerOptions: {
strict: true
Expand Down
11 changes: 11 additions & 0 deletions packages/svelte/src/.eslintrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"rules": {
"id-match": [
"error",
"^\\$\\$$|^[_$]?[_$]?[a-z][a-z0-9]*(?:_[a-z]+)*(?:_\\d+)?$|^[A-Z][A-Z0-9]*(?:_[A-Z][A-Z0-9]*)*(?:_\\d+)?$|^(?:[A-Z][a-z0-9]*)+(?:_\\d+)?$",
{
"onlyDeclarations": true
}
]
}
}
12 changes: 6 additions & 6 deletions packages/svelte/src/compiler/compile/Component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { TraceMap, originalPositionFor } from '@jridgewell/trace-mapping';
import { TraceMap, originalPositionFor as original_position_for } from '@jridgewell/trace-mapping';
import { walk } from 'estree-walker';
import { getLocator } from 'locate-character';
import { getLocator as get_locator } from 'locate-character';
import { reserved, is_valid } from '../utils/names.js';
import globals from '../utils/globals.js';
import { namespaces, valid_namespaces } from '../utils/namespaces.js';
Expand Down Expand Up @@ -213,7 +213,7 @@ export default class Component {
: compile_options.filename);

// line numbers in stack trace frames are 1-based. source maps are 0-based
this.locate = getLocator(this.source, { offsetLine: 1 });
this.locate = get_locator(this.source, { offsetLine: 1 });
/** @type {TraceMap | null | undefined} initialise lazy because only used in dev mode */
let tracer;
this.meta_locate = (c) => {
Expand All @@ -225,7 +225,7 @@ export default class Component {
}
if (tracer) {
// originalPositionFor returns 1-based lines like locator
location = originalPositionFor(tracer, location);
location = original_position_for(tracer, location);
}
return location;
};
Expand Down Expand Up @@ -1555,8 +1555,8 @@ export default class Component {
}, [])
);
if (cycle && cycle.length) {
const declarationList = lookup.get(cycle[0]);
const declaration = declarationList[0];
const declaration_list = lookup.get(cycle[0]);
const declaration = declaration_list[0];
return this.error(declaration.node, compiler_errors.cyclical_reactive_declaration(cycle));
}

Expand Down
12 changes: 6 additions & 6 deletions packages/svelte/src/compiler/compile/create_module.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ export default function create_module(

/**
* @param {any} source
* @param {any} sveltePath
* @param {any} svelte_path
*/
function edit_source(source, sveltePath) {
function edit_source(source, svelte_path) {
return source === 'svelte' || source.startsWith('svelte/')
? source.replace('svelte', sveltePath)
? source.replace('svelte', svelte_path)
: source;
}

Expand Down Expand Up @@ -84,7 +84,7 @@ function get_internal_globals(globals, helpers) {
* @param {any} program
* @param {import('estree').Identifier} name
* @param {string} banner
* @param {string} sveltePath
* @param {string} svelte_path
* @param {string} internal_path
* @param {Array<{ name: string; alias: import('estree').Identifier }>} helpers
* @param {Array<{ name: string; alias: import('estree').Identifier }>} globals
Expand All @@ -96,7 +96,7 @@ function esm(
program,
name,
banner,
sveltePath,
svelte_path,
internal_path,
helpers,
globals,
Expand All @@ -118,7 +118,7 @@ function esm(

/** @param {any} node */
function rewrite_import(node) {
const value = edit_source(node.source.value, sveltePath);
const value = edit_source(node.source.value, svelte_path);
if (node.source.value !== value) {
node.source.value = value;
node.source.raw = null;
Expand Down
4 changes: 2 additions & 2 deletions packages/svelte/src/compiler/compile/nodes/Binding.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export default class Binding extends Node {
this.is_readonly =
regex_dimensions.test(this.name) ||
regex_box_size.test(this.name) ||
(isElement(parent) &&
(is_element(parent) &&
((parent.is_media_node() && read_only_media_attributes.has(this.name)) ||
(parent.name === 'input' && type === 'file'))) /* TODO others? */;
}
Expand Down Expand Up @@ -127,6 +127,6 @@ export default class Binding extends Node {
* @param {import('./shared/Node.js').default} node
* @returns {node is import('./Element.js').default}
*/
function isElement(node) {
function is_element(node) {
return !!(/** @type {any} */ (node).is_media_node);
}
8 changes: 5 additions & 3 deletions packages/svelte/src/compiler/compile/nodes/EachBlock.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ export default class EachBlock extends AbstractBlock {
this.has_animation = false;
[this.const_tags, this.children] = get_const_tags(info.children, component, this, this);
if (this.has_animation) {
this.children = this.children.filter((child) => !isEmptyNode(child) && !isCommentNode(child));
this.children = this.children.filter(
(child) => !is_empty_node(child) && !is_comment_node(child)
);
if (this.children.length !== 1) {
const child = this.children.find(
(child) => !!(/** @type {import('./Element.js').default} */ (child).animation)
Expand All @@ -102,11 +104,11 @@ export default class EachBlock extends AbstractBlock {
}

/** @param {import('./interfaces.js').INode} node */
function isEmptyNode(node) {
function is_empty_node(node) {
return node.type === 'Text' && node.data.trim() === '';
}

/** @param {import('./interfaces.js').INode} node */
function isCommentNode(node) {
function is_comment_node(node) {
return node.type === 'Comment';
}
4 changes: 2 additions & 2 deletions packages/svelte/src/compiler/compile/nodes/shared/get_const_tags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ function sort_consts_nodes(consts_nodes, component) {
}, [])
);
if (cycle && cycle.length) {
const nodeList = lookup.get(cycle[0]);
const node = nodeList[0];
const node_list = lookup.get(cycle[0]);
const node = node_list[0];
component.error(node.node, compiler_errors.cyclical_const_tags(cycle));
}

Expand Down
23 changes: 12 additions & 11 deletions packages/svelte/src/compiler/compile/render_dom/wrappers/Window.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import Wrapper from './shared/Wrapper.js';
import { b, x } from 'code-red';
import add_event_handlers from './shared/add_event_handlers.js';
import add_actions from './shared/add_actions.js';

import EventHandler from './Element/EventHandler.js';
import Wrapper from './shared/Wrapper.js';
import add_actions from './shared/add_actions.js';
import add_event_handlers from './shared/add_event_handlers.js';

const associated_events = {
innerWidth: 'resize',
Expand Down Expand Up @@ -94,14 +95,14 @@ export default class WindowWrapper extends Wrapper {
bindings.scrollX && bindings.scrollY
? x`"${bindings.scrollX}" in this._state || "${bindings.scrollY}" in this._state`
: x`"${bindings.scrollX || bindings.scrollY}" in this._state`;
const scrollX = bindings.scrollX && x`this._state.${bindings.scrollX}`;
const scrollY = bindings.scrollY && x`this._state.${bindings.scrollY}`;
const scroll_x = bindings.scrollX && x`this._state.${bindings.scrollX}`;
const scroll_y = bindings.scrollY && x`this._state.${bindings.scrollY}`;
renderer.meta_bindings.push(b`
if (${condition}) {
@_scrollTo(${scrollX || '@_window.pageXOffset'}, ${scrollY || '@_window.pageYOffset'});
@_scrollTo(${scroll_x || '@_window.pageXOffset'}, ${scroll_y || '@_window.pageYOffset'});
}
${scrollX && `${scrollX} = @_window.pageXOffset;`}
${scrollY && `${scrollY} = @_window.pageYOffset;`}
${scroll_x && `${scroll_x} = @_window.pageXOffset;`}
${scroll_y && `${scroll_y} = @_window.pageYOffset;`}
`);
block.event_listeners.push(x`
@listen(@_window, "${event}", () => {
Expand Down Expand Up @@ -132,17 +133,17 @@ export default class WindowWrapper extends Wrapper {
// special case... might need to abstract this out if we add more special cases
if (bindings.scrollX || bindings.scrollY) {
const condition = renderer.dirty([bindings.scrollX, bindings.scrollY].filter(Boolean));
const scrollX = bindings.scrollX
const scroll_x = bindings.scrollX
? renderer.reference(bindings.scrollX)
: x`@_window.pageXOffset`;
const scrollY = bindings.scrollY
const scroll_y = bindings.scrollY
? renderer.reference(bindings.scrollY)
: x`@_window.pageYOffset`;
block.chunks.update.push(b`
if (${condition} && !${scrolling}) {
${scrolling} = true;
@_clearTimeout(${scrolling_timeout});
@_scrollTo(${scrollX}, ${scrollY});
@_scrollTo(${scroll_x}, ${scroll_y});
${scrolling_timeout} = @_setTimeout(${clear_scrolling}, 100);
}
`);
Expand Down
14 changes: 7 additions & 7 deletions packages/svelte/src/compiler/compile/utils/a11y.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { roles as roles_map, elementRoles } from 'aria-query';
import { AXObjects, AXObjectRoles, elementAXObjects } from 'axobject-query';
import { roles as roles_map, elementRoles as element_roles } from 'aria-query';
import { AXObjects, AXObjectRoles, elementAXObjects as element_ax_objects } from 'axobject-query';
import { regex_whitespaces } from '../../utils/patterns.js';

const aria_roles = roles_map.keys();
Expand Down Expand Up @@ -104,7 +104,7 @@ export function has_disabled_attribute(attribute_map) {
* @type {import('aria-query').ARIARoleRelationConcept[]}
*/
const non_interactive_element_role_schemas = [];
elementRoles.entries().forEach(([schema, roles]) => {
element_roles.entries().forEach(([schema, roles]) => {
if ([...roles].every((role) => role !== 'generic' && non_interactive_roles.has(role))) {
non_interactive_element_role_schemas.push(schema);
}
Expand All @@ -114,7 +114,7 @@ elementRoles.entries().forEach(([schema, roles]) => {
* @type {import('aria-query').ARIARoleRelationConcept[]}
*/
const interactive_element_role_schemas = [];
elementRoles.entries().forEach(([schema, roles]) => {
element_roles.entries().forEach(([schema, roles]) => {
if ([...roles].every((role) => interactive_roles.has(role))) {
interactive_element_role_schemas.push(schema);
}
Expand All @@ -132,7 +132,7 @@ const non_interactive_ax_objects = new Set(
* @type {import('aria-query').ARIARoleRelationConcept[]}
*/
const interactive_element_ax_object_schemas = [];
elementAXObjects.entries().forEach(([schema, ax_object]) => {
element_ax_objects.entries().forEach(([schema, ax_object]) => {
if ([...ax_object].every((role) => interactive_ax_objects.has(role))) {
interactive_element_ax_object_schemas.push(schema);
}
Expand All @@ -142,7 +142,7 @@ elementAXObjects.entries().forEach(([schema, ax_object]) => {
* @type {import('aria-query').ARIARoleRelationConcept[]}
*/
const non_interactive_element_ax_object_schemas = [];
elementAXObjects.entries().forEach(([schema, ax_object]) => {
element_ax_objects.entries().forEach(([schema, ax_object]) => {
if ([...ax_object].every((role) => non_interactive_ax_objects.has(role))) {
non_interactive_element_ax_object_schemas.push(schema);
}
Expand Down Expand Up @@ -241,7 +241,7 @@ export function is_static_element(tag_name, attribute_map) {
* @param {Map<string, import('../nodes/Attribute.js').default>} attribute_map
*/
export function is_semantic_role_element(role, tag_name, attribute_map) {
for (const [schema, ax_object] of elementAXObjects.entries()) {
for (const [schema, ax_object] of element_ax_objects.entries()) {
if (
schema.name === tag_name &&
(!schema.attributes ||
Expand Down
4 changes: 2 additions & 2 deletions packages/svelte/src/compiler/parse/read/context.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isIdentifierStart } from 'acorn';
import { isIdentifierStart as is_identifier_start } from 'acorn';
import full_char_code_at from '../../utils/full_char_code_at.js';
import {
is_bracket_open,
Expand All @@ -19,7 +19,7 @@ export default function read_context(parser) {
let i = parser.index;

const code = full_char_code_at(parser.template, i);
if (isIdentifierStart(code, true)) {
if (is_identifier_start(code, true)) {
return {
type: 'Identifier',
name: parser.read_identifier(),
Expand Down
4 changes: 2 additions & 2 deletions packages/svelte/src/compiler/parse/read/css-tree-cq/css_tree_parse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import * as node from './node/index.js';
*
* The new nodes are located in `./node`.
*/
const cqSyntax = fork({
const cq_syntax = fork({
atrule: {
// extend or override at-rule dictionary
container: {
Expand All @@ -25,4 +25,4 @@ const cqSyntax = fork({
node
});

export const parse = cqSyntax.parse;
export const parse = cq_syntax.parse;
6 changes: 3 additions & 3 deletions packages/svelte/src/compiler/parse/read/css-tree-cq/node/query_feature_range.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const structure = {
value: ['Identifier', 'Number', 'Comparison', 'Dimension', 'QueryCSSFunction', 'Ratio', null]
};

function lookup_non_WS_type_and_value(offset, type, referenceStr) {
function lookup_non_ws_type_and_value(offset, type, reference_str) {
let current_type;

do {
Expand All @@ -26,7 +26,7 @@ function lookup_non_WS_type_and_value(offset, type, referenceStr) {
}
} while (current_type !== 0); // NULL -> 0

return current_type === type ? this.lookupValue(offset - 1, referenceStr) : false;
return current_type === type ? this.lookupValue(offset - 1, reference_str) : false;
}

export function parse() {
Expand All @@ -40,7 +40,7 @@ export function parse() {
while (!this.eof && this.tokenType !== RightParenthesis) {
switch (this.tokenType) {
case Number:
if (lookup_non_WS_type_and_value.call(this, 1, Delim, '/')) {
if (lookup_non_ws_type_and_value.call(this, 1, Delim, '/')) {
child = this.Ratio();
} else {
child = this.Number();
Expand Down
7 changes: 5 additions & 2 deletions packages/svelte/src/compiler/utils/names.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { isIdentifierStart, isIdentifierChar } from 'acorn';
import {
isIdentifierStart as is_identifier_start,
isIdentifierChar as is_identifier_char
} from 'acorn';
import full_char_code_at from './full_char_code_at.js';
import { regex_starts_with_underscore, regex_ends_with_underscore } from './patterns.js';

Expand Down Expand Up @@ -62,7 +65,7 @@ export function is_valid(str) {

while (i < str.length) {
const code = full_char_code_at(str, i);
if (!(i === 0 ? isIdentifierStart : isIdentifierChar)(code, true)) return false;
if (!(i === 0 ? is_identifier_start : is_identifier_char)(code, true)) return false;

i += code <= 0xffff ? 1 : 2;
}
Expand Down
Loading