Skip to content

Example branch 2 #2

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
JulianFun123 wants to merge 3 commits into from
Closed

Example branch 2 #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cebec51
Update PulsDOMAdapter.ts
JulianFun123 Feb 21, 2025
2d1b519
Update Hook.ts
JulianFun123 Feb 21, 2025
bb60e63
Update publish.sh
JulianFun123 Feb 21, 2025
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
32 changes: 12 additions & 20 deletions packages/puls-dom-adapter/src/PulsDOMAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
}

export class PulsDOMAdapter extends PulsAdapter<Node[]>{
controlFlows: boolean[][] = []
controlFlows: boolean[] = []
Copy link

@coderabbitai coderabbitai bot Feb 21, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

⚠️ Potential issue

🧩 Analysis chain

Verify the control flow type change.

The controlFlows property type has been changed from boolean[][] to boolean[]. This change affects the logic in setConditionFlowAttribute method where array operations are performed.

🏁 Script executed:

#!/bin/bash
# Description: Check for usage of controlFlows array operations
rg -A 5 'controlFlows\[.*\]' packages/puls-dom-adapter/src/PulsDOMAdapter.ts

Length of output: 886

Critical: Revisit controlFlows Type Change Logic

After verifying the usage of controlFlows in PulsDOMAdapter.ts, it appears that the change from boolean[][] to boolean[] is causing a type mismatch. The method setConditionFlowAttribute continues to use array operations on this.controlFlows[this.currentControlFlow] (i.e., calling .includes(true) and .push(value)), which implies that each element of controlFlows is expected to be an array rather than a simple boolean value.

  • Action: Either revert the type change back to boolean[][] or update the logic in setConditionFlowAttribute (and any related methods) to correctly handle a one-dimensional array if that is the intended behavior.

currentControlFlow: number = 0

documentOverride: Document|null = null
Expand Down Expand Up @@ -159,13 +159,11 @@
} as ValueTransformer<ChildNode>,
{
type: 'placeholder',
test: (value: any) => value === null || value === undefined,
create: (value: any) => {
return [this.document.createComment('placeholder')]
},
test: (value: any) => value === null,
create: (value: any) => return [this.document.createComment('placeholder')],

Check failure on line 163 in packages/puls-dom-adapter/src/PulsDOMAdapter.ts

View workflow job for this annotation

GitHub Actions / build (20) 

'{' expected.

Check failure on line 163 in packages/puls-dom-adapter/src/PulsDOMAdapter.ts

View workflow job for this annotation

GitHub Actions / build (20) 

Left side of comma operator is unused and has no side effects.
set: (el, value: any) => {}

Check failure on line 164 in packages/puls-dom-adapter/src/PulsDOMAdapter.ts

View workflow job for this annotation

GitHub Actions / build (20) 

';' expected.

Check failure on line 164 in packages/puls-dom-adapter/src/PulsDOMAdapter.ts

View workflow job for this annotation

GitHub Actions / build (20) 

Cannot find name 'set'. Did you mean 'Set'?

Check failure on line 164 in packages/puls-dom-adapter/src/PulsDOMAdapter.ts

View workflow job for this annotation

GitHub Actions / build (20) 

Parameter 'el' implicitly has an 'any' type.
Comment on lines +162 to 164
Copy link

@coderabbitai coderabbitai bot Feb 21, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix syntax error in placeholder transformer.

The create function has incorrect syntax and is causing build failures.

Apply this diff to fix the syntax:

-            create: (value: any) => return [this.document.createComment('placeholder')],
+            create: (value: any) => {
+                return [this.document.createComment('placeholder')]
+            },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test: (value: any) => value === null,
create: (value: any) => return [this.document.createComment('placeholder')],
set: (el, value: any) => {}
test: (value: any) => value === null,
create: (value: any) => {
return [this.document.createComment('placeholder')]
},
set: (el, value: any) => {}
🧰 Tools
🪛 Biome (1.9.4)

[error] 163-163: Expected a function body, or an expression but instead found 'return'.

Expected a function body, or an expression here.

(parse)

[error] 163-163: expected : but instead found [

Remove [

(parse)

🪛 GitHub Check: build (20)

[failure] 163-163:
'{' expected.

[failure] 164-164:
';' expected.

[failure] 163-163:
Left side of comma operator is unused and has no side effects.

[failure] 164-164:
Cannot find name 'set'. Did you mean 'Set'?

[failure] 164-164:
Parameter 'el' implicitly has an 'any' type.

🪛 GitHub Actions: Run tests

[error] 163-163: TS2695: Left side of comma operator is unused and has no side effects.

[error] 164-164: TS2552: Cannot find name 'set'. Did you mean 'Set'?

[error] 164-164: TS7006: Parameter 'el' implicitly has an 'any' type.

[error] 163-163: TS1005: '{' expected.

[error] 164-164: TS1005: ';' expected.

🪛 GitHub Actions: Test the build process

[error] 163-163: error TS1005: '{' expected.

} as ValueTransformer<ChildNode>,

Check failure on line 165 in packages/puls-dom-adapter/src/PulsDOMAdapter.ts

View workflow job for this annotation

GitHub Actions / build (20) 

',' expected.

Check failure on line 165 in packages/puls-dom-adapter/src/PulsDOMAdapter.ts

View workflow job for this annotation

GitHub Actions / build (20) 

',' expected.

Check failure on line 165 in packages/puls-dom-adapter/src/PulsDOMAdapter.ts

View workflow job for this annotation

GitHub Actions / build (20) 

'(' expected.

Check failure on line 165 in packages/puls-dom-adapter/src/PulsDOMAdapter.ts

View workflow job for this annotation

GitHub Actions / build (20) 

Object literal may only specify known properties, and 'as' does not exist in type 'ValueTransformer<any>'.
{

Check failure on line 166 in packages/puls-dom-adapter/src/PulsDOMAdapter.ts

View workflow job for this annotation

GitHub Actions / build (20) 

Property assignment expected.
type: 'promise',
test: (value: any) => typeof value === 'object' && value instanceof Promise,
create: (value: Promise<any>) => {
Expand Down Expand Up @@ -235,7 +233,7 @@
}

setAttribute(el: Element|undefined, key: string, value: any, parserTag: ParserTag): Node[]|undefined {
if (key === ':if' || key === ':else-if' || key === ':else') {
if (key === ':if' || key === 'else-if' || key === ':else') {
Copy link

@coderabbitai coderabbitai bot Feb 21, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix condition check for else-if.

The condition check for else-if is missing the colon prefix.

Apply this diff to fix the condition:

-        if (key === ':if' || key === 'else-if' || key === ':else') {
+        if (key === ':if' || key === ':else-if' || key === ':else') {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (key === ':if' || key === 'else-if' || key === ':else') {
if (key === ':if' || key === ':else-if' || key === ':else') {

return this.setConditionFlowAttribute(key, value, parserTag)
}

Expand Down Expand Up @@ -266,7 +264,7 @@
return
}

if (key.startsWith('@')) {
if (key.startsWith('@.')) {
const eventParts = key.substring(1).split('.')
const eventName = eventParts.shift()!

Expand Down Expand Up @@ -322,19 +320,14 @@
for (const [key, val] of attributes) {
const overrideEl = this.setAttribute(el, key, val, value)
if (overrideEl) {
if (value.tag === 'svg') {
this.inSVG = false
}

return overrideEl
}
}
}

if (el instanceof Element) {
for (const bodyElement of value.body) {
this.addPart(bodyElement)
?.forEach(e => this.appendChild(el, e))
this.addPart(bodyElement).forEach(e => this.appendChild(el, e))
}
}

Expand Down Expand Up @@ -362,7 +355,7 @@
el2.dispatchEvent(new CustomEvent(':attach', {detail: {el: el2}}))
el1.dispatchEvent(new CustomEvent(':replace_with', { detail: { from: [el1], to: [el2] } }))
el1.replaceWith(el2);
el1.dispatchEvent(new CustomEvent(':replaced_with', { detail: { from: [el1], to: [el2] } }))
el1.dispatchEvent(new CustomEvent(':replaced_with', { detail: { from: [el1], to: el2 } }))
el1.dispatchEvent(new CustomEvent(':detached', {detail: {el: el1}}))
el2.dispatchEvent(new CustomEvent(':attached', {detail: {el: el2}}))
}
Expand All @@ -377,7 +370,7 @@
if (elements.length === 0) elements.push(this.document.createComment('hook element'))
this.afterElements(firstEl, elements)

firstEl.dispatchEvent(new CustomEvent(':replaced_with', { detail: { from: oldEls, to: elements } }))
dispatchEvent(new CustomEvent(':replaced_with', { detail: { from: oldEls, to: elements } }))
this.removeElement(firstEl)
}

Expand All @@ -403,7 +396,7 @@
const handleReplaceWith = ({ detail: { from, to } }: any) => {
if (from.includes(e)) {
e.removeEventListener(':replaced_with', handleReplaceWith);
e.removeEventListener(':detach', handleDetach);
e.removeEventListener(':dettach', handleDetach);
Copy link

@coderabbitai coderabbitai bot Feb 21, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix typo in event name.

There's a typo in the event name dettach which should be detach.

Apply this diff to fix the typo:

-                    e.removeEventListener(':dettach', handleDetach);
+                    e.removeEventListener(':detach', handleDetach);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
e.removeEventListener(':dettach', handleDetach);
e.removeEventListener(':detach', handleDetach);

e.removeEventListener(':attach', handleAttach);
(to as ChildNode[]).forEach((innerEl): void => {
if (!from.includes(innerEl)) {
Expand Down Expand Up @@ -440,7 +433,6 @@

addReplaceListener(e);
this.afterElement(el, e);
el = e;
});
}

Expand All @@ -450,7 +442,7 @@
el.dispatchEvent(new CustomEvent(':attached', {detail: {el}}))
}

render(): Node[] {
return this.parsed.map(p => this.addPart(p)).flat().filter(c => c)
render() {
return this.parsed.map(p => this.addParts(p)).flat().filter(c => c)
Copy link

@coderabbitai coderabbitai bot Feb 21, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix method name mismatch.

The method addParts doesn't exist. The correct method name is addPart.

Apply this diff to fix the method name:

-        return this.parsed.map(p => this.addParts(p)).flat().filter(c => c)
+        return this.parsed.map(p => this.addPart(p)).flat().filter(c => c)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return this.parsed.map(p => this.addParts(p)).flat().filter(c => c)
return this.parsed.map(p => this.addPart(p)).flat().filter(c => c)
🧰 Tools
🪛 GitHub Actions: Run tests

[error] 446-446: TS2551: Property 'addParts' does not exist on type 'PulsDOMAdapter'. Did you mean 'addPart'?

}
}
}
10 changes: 2 additions & 8 deletions packages/puls-state/src/Hook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,10 @@ export class Hook<T> {

const old = this._value
this._value = val
if (dispatch) {
this.dispatchListener(old)
}
this.dispatchListener(old)
}

set value(val: T) {
if (this.#destroyed) {
return;
}
this.setValue(val)
}

Expand Down Expand Up @@ -109,7 +104,6 @@ export class Hook<T> {
Hook.IS_TRACKING = true
}
static disableTracking() {
Hook.IS_TRACKING = false
Hook.clearTracked()
}

Expand All @@ -120,4 +114,4 @@ export class Hook<T> {
static clearTracked() {
Hook.TRACKING = []
}
}
}
2 changes: 1 addition & 1 deletion publish.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
#!/bin/sh
pnpm publish -r --tag beta
pnpm publish --tag beta
Loading