chore(CI): reenable test:projects:cra-ts
#608
Conversation
layershifter
requested review from
dvdzkwsk,
kuzhelov,
levithomason,
miroslavstastny and
mnajdova
as code owners
build/gulp/tasks/test-projects.tsx
Outdated
| @@ -167,7 +124,7 @@ task('test:projects:rollup', async () => { | |||
| task( | |||
| 'test:projects', | |||
| series( | |||
| // 'test:projects:cra-ts', Temporary disabled | |||
| 'test:projects:rollup', | |||
| series('build:dist', 'test:projects:pack'), | |||
There was a problem hiding this comment.
yarn gulp build:dist and yarn build:dist do the same work, if we do it there as Gulp's we will avoid double transpilation with ts-node 🚀
| } | ||
|
|
||
| export default App; | ||
| ` |
There was a problem hiding this comment.
We have now a separate folder for this 👍
build/gulp/tasks/test-projects.tsx
Outdated
| await sh(`yarn pack --filename ${packageFilename}`) | ||
|
|
||
| logger(`Stardust package is published: ${packageFilename}`) | ||
| }) |
There was a problem hiding this comment.
We will run test:projects:pack only once for both tests 🚀
Generated by 🚫 dangerJS |
| @@ -9,23 +9,12 @@ import config from '../../../config' | |||
| import * as tmp from 'tmp' | |||
|
|
|||
| const { paths } = config | |||
|
|
|||
| const log = msg => { | |||
There was a problem hiding this comment.
now the output is indistinguishable, it is very hard to understand what is going on and immediately find the step necessary:
It will significantly complicate debugging scenarios
There was a problem hiding this comment.
Agree, I restored your logger
build/gulp/tasks/test-projects.tsx
Outdated
| console.log('CRA TS:', msg) | ||
| console.log('='.repeat(80)) | ||
| } | ||
| let packageFilename |
There was a problem hiding this comment.
don't like this broken encapsulation between the tasks, especially given that all can be tried to use in isolation. This implicit composability assumptions would be really hard to track - thus, would prefer correctness there at the first place
There was a problem hiding this comment.
I'm not so familiar with Gulp, do you have a better approach there? How I can pass a name between tasks?
There was a problem hiding this comment.
Nevermind, yarn pack is quite fast so I moved it back to each step.
There was a problem hiding this comment.
@layershifter, thanks - unfortunately, there is built-in support in gulp to produce result from task (just in a form of artefacts, but this is definitely not what your question about:) As alternative, we might consider cache semantics, but absolutely agree that it is better to split, especially if this is a quick step 👍
…ore/reenable-project-test
|
just note aside - could you, please, suggest if you've had a chance to research the cause (source) of these TS warnings:
I mean, I see that those are produced by rollup's env, but just to understand which problem this is. Thanks! |
This problem caused by TS helpers in the transpiled code, more details are there: rollup/rollup#794 (comment). I am not sure that we need to silence this warning, especially if we want to migrate to Babel. |
This PR reenables previously disabled
test:projects:cra-ts, #549.