-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
v2 #1348
v2 #1348
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1348 +/- ##
==========================================
- Coverage 91.69% 91.53% -0.17%
==========================================
Files 127 127
Lines 4542 4464 -78
Branches 1446 1381 -65
==========================================
- Hits 4165 4086 -79
- Misses 151 153 +2
+ Partials 226 225 -1
Continue to review full report at Codecov.
|
test/parser/index.js
Outdated
@@ -2,7 +2,7 @@ import assert from 'assert'; | |||
import fs from 'fs'; | |||
import { svelte, tryToLoadJson } from '../helpers.js'; | |||
|
|||
describe('parse', () => { | |||
describe.only('parse', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
D:
test/runtime/index.js
Outdated
return base[0].toUpperCase() + base.slice(1); | ||
} | ||
|
||
describe("runtime", () => { | ||
describe.only("runtime", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
D: D:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
argh
There's one really awkward failing test remaining. Trying to figure out the precise behaviour around initialisation and It makes sense for During the initial render, it's possible for components with bindings to modify the parent state object. But we don't want (Note that I'm not describing a regression; the test in question was previously checking that bindings were flushed before One valid option is to worry about this post-v2 (i.e. treat the current behaviour as a minor bug), if it's too hard to figure out right now. |
I can't do any further work on this right now, but I believe there's just two remaining tasks:
|
I think that's everything? Will prepare a PR for the docs site. |
The readme needs some updates - the structure of the return value of |
The |
Pushed a commit to hopefully fix the above issues with |
window.scrollTo(window.pageXOffset, y); | ||
window_updating_timeout = setTimeout(clear_window_updating, 100); | ||
component.on("state", ({ changed, current }) => { | ||
if (changed["y"]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason for preferring changed["y"]
over changed.y
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rely on the names of the bindings to be safe for use as property names? I was just trying to be super safe. If we can rely on that, or if there is some nice codegen utility function somewhere already that takes care of that, I'd support changing this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, I thought there was a utility for that but it seems we're just doing this everywhere instead...
generator.legacy ? `["default"]` : `.default`
(which reminds me — need to remove that)
here it comes...