Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make things async wherever possible #265

Closed
jaredpalmer opened this issue Oct 17, 2019 · 7 comments
Closed

Make things async wherever possible #265

jaredpalmer opened this issue Oct 17, 2019 · 7 comments
Labels
good first issue Good for newcomers scope: internal Changes only affect the internal API

Comments

@jaredpalmer
Copy link
Owner

jaredpalmer commented Oct 17, 2019

I noticed some sync file system tasks slipped into .10.x. We should make these async wherever possible.

Example: we should make this async

if (fs.existsSync(paths.appDist)) {

@jaredpalmer jaredpalmer added the good first issue Good for newcomers label Oct 17, 2019
@gndelia
Copy link
Contributor

gndelia commented Oct 21, 2019

I would like to work on this one 😄

@gndelia
Copy link
Contributor

gndelia commented Oct 21, 2019

Do I need to be formally assigned or should I start working on it directly?

@swyxio
Copy link
Collaborator

swyxio commented Oct 22, 2019

go ahead!

@gndelia
Copy link
Contributor

gndelia commented Oct 29, 2019

Hi
I have a draft PR, however I am having issues to commit - I am getting eslint errors under test/tests/lint - although that folder should be excluded. I get them even without my changes

I am running yarn lint (actually it is being run in the pre-commit hook)

$ yarn && yarn lint
yarn install v1.19.1
[1/4] Resolving packages...
success Already up-to-date.
$ tsc -p tsconfig.json
Done in 3.86s.
yarn run v1.19.1
$ yarn build && yarn lint:post-build
$ tsc -p tsconfig.json
$ node dist/index.js lint src test --ignore-pattern 'test/tests/lint'

C:\Users\DELL\Dev\Personal\tsdx\test\tests\lint\file-with-lint-errors.ts
  1:17  error  Parsing error: ',' expected

C:\Users\DELL\Dev\Personal\tsdx\test\tests\lint\file-with-prettier-lint-errors.ts
  1:18  error  Replace `·=·(···)·=>·⏎!!·('bar')⏎` with `=·()·=>·!!'bar'`  prettier/prettier
  4:1   error  Delete `⏎·`                                                prettier/prettier

C:\Users\DELL\Dev\Personal\tsdx\test\tests\lint\react-file-with-lint-errors.tsx
  4:9  error  Parsing error: Expression expected

✖ 4 problems (4 errors, 0 warnings)
  2 errors and 0 warnings potentially fixable with the `--fix` option.

Any ideas? My understanding is that those fields should be excluded from being analyzed

edit: nvm, I found the problem... windows 😆 I had to locally replace the single quotes for the path --ignore-pattern with escaped double quotes and it worked

(should I create an issue for that?)

@swyxio
Copy link
Collaborator

swyxio commented Nov 4, 2019

@gndelia lets address those windows lint issues separately, and get this PR in as soon as you feel it is ready.

@swyxio
Copy link
Collaborator

swyxio commented Dec 4, 2019

some progress made on these thanks to @gndelia!

@agilgur5 agilgur5 added the scope: internal Changes only affect the internal API label Mar 9, 2020
@agilgur5
Copy link
Collaborator

agilgur5 commented Mar 9, 2020

See #291 . I did a bit on this with by using fs.pathExists from fs-extra instead of the old fs.existsSync, and some other things IIRC. But I think there's a tiny bit more left to fix that I'll get to soon too.

@agilgur5 agilgur5 closed this as completed Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers scope: internal Changes only affect the internal API
Projects
None yet
Development

No branches or pull requests

4 participants