Extract validation system and tags into @glimmer/validator package
#987
Conversation
pzuraq
force-pushed
the
refactor/isolate-tags
branch
2 times, most recently
from
0d09a4f
to
1566b06
Compare
This PR extracts the tag and autotracking portions of the `@glimmer/reference` package, which are fairly isolated from the rest of the VM, and removes any additional dependencies they have on other VM packages. This keeps them isolated, which makes it less likely that type changes or issues could leak and cause problems in downstream packages which have public types, such as `@glimmer/tracking`.
pzuraq
force-pushed
the
refactor/isolate-tags
branch
from
1566b06
to
415c374
Compare
| import { Slice, LinkedListNode } from '@glimmer/util'; | ||
| import { Tag, Tagged, createCombinatorTag, CONSTANT_TAG } from '@glimmer/tag'; | ||
|
|
||
| export function combineTagged(tagged: ReadonlyArray<Tagged>): Tag { |
There was a problem hiding this comment.
Why is this in runtime and not tag?
There was a problem hiding this comment.
I wanted to avoid pulling extra types into @glimmer/tag for things that are essentially unrelated. These combinators are really optimizations for a couple of narrow use cases in the VM itself, and while I wasn't comfortable removing them/simplifying them without perf tests, I also don't think they really belong with the rest of the tracking infra, conceptually.
If perf wasn't a concern, or tests showed it wasn't an issue, I probably would have made them use the combine() function like everyone else. If perf is an issue, I think the internal createCombinatorTag API is good for solving the problem.
| @@ -0,0 +1,107 @@ | |||
| import { Tag, combine, CONSTANT_TAG } from './tags'; | |||
| import { createTag, dirty } from './tags'; | |||
There was a problem hiding this comment.
Seems vaguely odd for tag package to include tracked. Maybe “tag” isn’t the right name? Any other options?
pzuraq
force-pushed
the
refactor/isolate-tags
branch
from
d67729f
to
48349a6
Compare
pzuraq
changed the title
@glimmer/tag@glimmer/validator
|
@pzuraq - Would you mind leaving a little info in this PR description about how you expect Glimmer.js and Ember.js to absorb the breakage (might just be update import paths, but I dunno)? Just something new I'm trying to do to aid the upgrade... |
rwjblue
changed the title
@glimmer/validator@glimmer/validator package
This PR extracts the tag and autotracking portions of the
@glimmer/referencepackage, which are fairly isolated from the restof the VM, and removes any additional dependencies they have on other VM
packages. This keeps them isolated, which makes it less likely that
type changes or issues could leak and cause problems in downstream
packages which have public types, such as
@glimmer/tracking.Also updates the implementation of autotracking in the VM to match Ember's directly, so that we can add this package once Ember is able to update its VM.
Impact on Ember.js and Glimmer.js
@glimmer/validator. Reference imports can remain the same.metasystem, and should be stored solely using the meta provided by@glimmer/validator.