Service Registration: Chunk #7880
Conversation
src/chunk.js
Outdated
| @@ -107,7 +123,7 @@ export function activateChunkingForTesting() { | |||
| * @param {!Node|!./service/ampdoc-impl.AmpDoc} nodeOrAmpDoc | |||
| */ | |||
| export function runChunksForTesting(nodeOrAmpDoc) { | |||
| const service = fromClassForDoc(nodeOrAmpDoc, 'chunk', Chunks); | |||
| const service = getChunkServiceForDoc_(nodeOrAmpDoc); | |||
src/chunk.js
Outdated
| * @param {!Node|!./service/ampdoc-impl.AmpDoc} nodeOrAmpDoc | ||
| * @return {!Chunks} | ||
| */ | ||
| function getChunkServiceForDoc_(nodeOrAmpDoc) { |
There was a problem hiding this comment.
You can remove this method if you register the service in startupChunk. Registering and retrieving a service in the same method still looks weird to me.
There was a problem hiding this comment.
The function is called in three places. I'm not sure I see the value added by inlining it.
src/chunk.js
Outdated
| /** | ||
| * @param {!Node|!./service/ampdoc-impl.AmpDoc} nodeOrAmpDoc | ||
| */ | ||
| export function installChunkServiceForDoc_(nodeOrAmpDoc) { |
There was a problem hiding this comment.
Does this need to be exported? Note that trailing underscore is a naming convention for private props/functions.
If it's called in only one place, you probably don't need the method to start with.
|
Hi, ampproject bot here! Here are a list of the owners that can approve your files. You may leave an issue comment stating "@ampprojectbot retry!" to force me to re-evaluate this Pull Request's status /to dvoytenko jridgewell
For any issues please file a bug at https://github.com/google/github-owners-bot/issues |
| @@ -37,6 +37,15 @@ let deactivated = /nochunking=1/.test(self.location.hash); | |||
| const resolved = Promise.resolve(); | |||
|
|
|||
| /** | |||
| * @param {!Node|!./service/ampdoc-impl.AmpDoc} nodeOrAmpDoc | |||
| * @return {!Chunks} | |||
kmh287
force-pushed
the
service_registration15_chunk
branch
from
6a9631d
to
9de9664
Compare
This pull request distinguishes installing/registering the chunk service and creating it.Please see #7839 for more context.
/to @choumx @jridgewell