Rework how we handle laziness. #543
Conversation
|
Part of handling Polymer/tools-sample-projects#3 |
src/model/queryable.ts
Outdated
| * Do not include any features that are only reachable via paths that include | ||
| * lazy import edges. | ||
| */ | ||
| strictImports?: boolean; |
There was a problem hiding this comment.
can we rename this to lazyImports with default true?
There was a problem hiding this comment.
I'd been going with the idea that everything was default false, to match the semantics of options.foo without explicit defaults handling stuff.
There was a problem hiding this comment.
that's certainly a bit simpler, but the name here doesn't have a clear meaning to me. How about a convention of a "no" prefix for options we'd rather have a default of true. So noLazyImports?
| const imports = scannedDocument.getNestedFeatures().filter( | ||
| (e) => e instanceof ScannedImport && | ||
| e.type !== 'lazy-html-import') as ScannedImport[]; | ||
| (e) => e instanceof ScannedImport) as ScannedImport[]; |
There was a problem hiding this comment.
I thought there were more places that needed fixup...
There was a problem hiding this comment.
Good catch, got the other place in analysis-context
src/html/html-import-scanner.ts
Outdated
| } else { | ||
| return; | ||
| } | ||
| const type = 'html-import'; |
There was a problem hiding this comment.
no longer need type variable here, only used once below
src/model/document.ts
Outdated
| @@ -238,14 +238,15 @@ export class Document implements Feature, Queryable { | |||
| getByKind(kind: string, options?: QueryOptions): Set<Feature>; | |||
| getByKind(kind: string, options?: QueryOptions): Set<Feature> { | |||
| options = options || {}; | |||
| if (this._featuresByKind && options.imported) { | |||
| if (this._featuresByKind && options.imported && !options.strictImports) { | |||
There was a problem hiding this comment.
!options.strictImports is used in both checks below, can you pull it out of both and handle it separately?
There was a problem hiding this comment.
+1, this was a good clarity change
|
This is approved so why has it not been merged yet? Was about to submit an almost identical PR before I saw this one... |
|
I wanted to get feedback from Justin, but he's going on vacation, lemmy ping him, get his thoughts |
rictic
force-pushed
the
lazy-imports
branch
3 times, most recently
from
3459f19
to
10536c7
Compare
TODO: test strict querying.
TODO: test strict querying.
/cc @usergenic as this affects bundling.