feat(storefront): bctheme-1064 Modify Stencil bundle for using Components UI Library #904
Conversation
yurytut1993
force-pushed
the
BCTHEME-1064
branch
2 times, most recently
from
96094e8 to
c440b56
Compare
| @@ -6,6 +6,44 @@ const upath = require('upath'); | |||
|
|
|||
| const partialRegex = /\{\{>\s*([_|\-|a-zA-Z0-9/]+)[^{]*?}}/g; | |||
| const dynamicComponentRegex = /\{\{\s*?dynamicComponent\s*(?:'|")([_|\-|a-zA-Z0-9/]+)(?:'|").*?}}/g; | |||
| const includeRegex = /{{2}>\s*([_|\-|a-zA-Z0-9/]+)[^{]*?}{2}/g; | |||
| const packageMarker = 'external/'; | |||
There was a problem hiding this comment.
Maybe it's better to consolidate this name with externalTemplatesPath in stencil-bundle?
There was a problem hiding this comment.
We removed externalTemplatesPath
If your comment is still valid please can you add more details:)
| const includeRegex = /{{2}>\s*([_|\-|a-zA-Z0-9/]+)[^{]*?}{2}/g; | ||
| const packageMarker = 'external/'; | ||
|
|
||
| const isExternalTemplate = (templateName) => { |
There was a problem hiding this comment.
Could you please add here and below doc types?
lib/stencil-bundle.js
Outdated
| console.log('Template Parsing Started...'); | ||
| const internalTemplatesList = await recursiveReadDir(this.templatesPath, ['!*.html']); | ||
| const searchExternalLib = (content) => { |
There was a problem hiding this comment.
Do you mind moving searchExternalLib and checkTemplate to class functions (and maybe renaming to make more sense in class domain).
Maybe you can think also about extracting this functionality into separate file, while you are here?
lib/stencil-bundle.js
Outdated
| } | ||
| let externalLibs; | ||
| try { | ||
| externalLibs = (await async.map(internalTemplatesList, checkTemplate)).flat(); |
There was a problem hiding this comment.
I would recommend not to use async library, where you can implement it in vanilla js
lib/stencil-bundle.js
Outdated
| }); | ||
|
|
||
| // eslint-disable-next-line no-unused-vars | ||
| const checkResult = await async.parallel([ |
There was a problem hiding this comment.
Here also. I know, that it was in the legacy code, but while you are working on this part, it would really helpful to reduce dependency from async
|
|
||
| console.log(`${'ok'.green} -- Template Parsing Finished`); | ||
|
|
||
| return ret; |
There was a problem hiding this comment.
did you try to use return callback(null, ret) here? What happened?
There was a problem hiding this comment.
we received task object with field templates undefined which prevented us to create bundle
lib/stencil-bundle.js
Outdated
| ); | ||
| } | ||
| async | ||
| .series(this.tasks) |
There was a problem hiding this comment.
Is it because you removed the callback invocation?
There was a problem hiding this comment.
returned parallel function
lib/stencil-bundle.js
Outdated
|
|
||
| return res; | ||
| }; | ||
| return externalTemplatesImports.map((_import) => _import.split('/')[1]); |
There was a problem hiding this comment.
Is there a reason using underscore in _import?
There was a problem hiding this comment.
do you have any good name suggestions?
There was a problem hiding this comment.
I mean, that's an old style to declare private class members, while this is not a class and not a member of it
yurytut1993
force-pushed
the
BCTHEME-1064
branch
from
01f1f0b to
59af216
Compare
What?
This PR provides functionality of adding templates from external lib to the theme bundle. The main idea is that script can understand is used template consist in the theme or inside external node_modules library.
IMPORTANT
It should be merged only after bctheme-1063
Tickets / Documentation
bctheme-1064
Screenshots (if appropriate)
TBD