docs: Update migration howto with jest @src alias#182
docs: Update migration howto with jest @src alias#182arbrandes wants to merge 1 commit intoopenedx:mainfrom
Conversation
bc1fbb7 to
8273152
Compare
8273152 to
7205845
Compare
brian-smith-tcril
left a comment
There was a problem hiding this comment.
Is there a way to have a single source of truth for asset file types? It looks like we used to just have a big list in moduleNameMapper, but now we need to call out file types in the build target too.
I don't think this is something frontend-base should maintain, each app can specify what file types are used in its assets, but needing to update that list in 2 places feels less than ideal.
docs/how_tos/migrate-frontend-app.md
Outdated
| tsc-alias -p tsconfig.build.json | ||
| ``` | ||
|
|
||
| Note that if you import assets such as SVG files using aliases, you have to call `tsc-alias` after the assets are copied to `dist/`. Otherwise, it won't find them and omit them from the relative path conversion. |
There was a problem hiding this comment.
Might be good to mention the build target in the example makefile is doing this for png and svg. That should make it clearer that this note is saying "if you have asset files using types that aren't covered by the example build target, you can update the build target to include those types"
7205845 to
09936f4
Compare
The migration howto was missing the @src alias in jest configuration, since it's no longer provided by the default in frontend-base. Add the finding that `tsc-alias` needs to be called after assets are copied. Finally, document the fix to the site.config.test.tsx circular dependency when mocking `@openedx/frontend-base` itself.
09936f4 to
5753c56
Compare
I think it's just happenstance that the particular assets we want to copy over to dist are also ones that require a jestism to mock out. Another way to put it is: would you trust having the Makefile parse jest.config.js and then copy assets based on that? I couldn't agree more with the following, though:
How about we just assume what sane app authors would want to do anyway, which is to put all assets under a predictable directory name? That way the jestism remains a jestism (no way around that), but they don't have to worry about the Makefile. See changes in the latest commit. |
holaontiveros
left a comment
There was a problem hiding this comment.
Haven't seen how the fail with the circular dependency works, and the solution is perfect but maybe it should be somewhere else apart from the doc so it's easily recognizable for anyone that it shouldn't like that in there.
Also maybe we need to change it in here? ->
frontend-base/shell/site.config.test.tsx
Line 11 in cb555ff
|
@holaontiveros, I don't think it's needed in the shell because the shell can't mock the entire |
|
That makes sense, but then wouldn't that confuse someone that only sees that one and tries to use it as a refernce? |
They should be using frontend-template-app@frontend-base for reference... Once we update it. :) For now, the migration howto is the reference. |
The migration howto was missing the @src alias in jest configuration, since it's no longer provided by the default in frontend-base.
The
site.config.test.tsxcircular import fix was also included.