fs.existsSync deprecated, replace with exists-sync #4316
Conversation
jasonmit
changed the title
| @@ -26,10 +27,10 @@ function downloaded(item) { | |||
| var exists = false; | |||
| switch (item) { | |||
| case 'node_modules': | |||
| exists = fs.existsSync(path.join(root, '.node_modules-tmp')); | |||
| exists = exists(path.join(root, '.node_modules-tmp')); | |||
There was a problem hiding this comment.
Will need to change these so you're not overwriting exists. :)
|
I've updated the issue to reflect. |
Yup, I agree. |
| @@ -392,7 +393,7 @@ describe('Acceptance: brocfile-smoke-test', function() { | |||
|
|
|||
| return runCommand(path.join('.', 'node_modules', 'ember-cli', 'bin', 'ember'), 'build', '--silent') | |||
| .then(function() { | |||
| var exists = fs.existsSync(path.join('.', 'dist', 'assets', appName + '.css')); | |||
| var exists = exists(path.join('.', 'dist', 'assets', appName + '.css')); | |||
There was a problem hiding this comment.
Looks like another one here, but it won't matter when you've changed everything to use existsSync instead.
LGTM 👍 |
|
Squashed, thanks @trabus for the review |
|
Looks good. That file-utile relative import should likely just point to the actual existsSync no? |
|
@stefanpenner yes and should already be doing that in this PR |
| @@ -12,6 +12,7 @@ var runCommand = require('../helpers/run-command'); | |||
| var acceptance = require('../helpers/acceptance'); | |||
| var copyFixtureFiles = require('../helpers/copy-fixture-files'); | |||
| var assertDirEmpty = require('../helpers/assert-dir-empty'); | |||
| var existsSync = require('../helpers/file-utils').existsSync; | |||
There was a problem hiding this comment.
@jasonmit what about these?
They appear to use fs-extra's https://github.com/ember-cli/ember-cli/blob/master/tests/helpers/file-utils.js#L5-L7
There was a problem hiding this comment.
file-utils uses exists-sync now https://github.com/jasonmit/ember-cli/blob/exist-sync/tests/helpers/file-utils.js#L4
Perhaps I'm misunderstanding
There was a problem hiding this comment.
yes, but why bother with the intermediate file at all?
There was a problem hiding this comment.
Okay, will change this shortly
|
@stefanpenner updated to remove intermediate file |
fs.existsSync deprecated, replace with exists-sync
|
Awesome thanks |
|
@jasonmit Thanks for taking care of this! |
#4315
I took a first pass at this, noticed the tests should likely be using tests/helpers/file-utils where possible for exists. Want me to address this as well @stefanpennerI went ahead and updated the tests to use
file-utils.existsI will squash once thumbs up