feat(api): add Node-compatible fs module #10998

sgtcoolguy · 2019-06-26T21:05:17Z

JIRA: https://jira.appcelerator.org/browse/TIMOB-18583

Description:
This adds a node-compatible fs module in our common JS code shipped with the SDK.

The goal is to have a compatible shim that allows use of more npm packages out there since this is a pretty heavily used core module of Node. When possible I attempted to make the async variants more than simple wrappers that scheduled the sync version in a setTimeout() call (though I used that pattern for a number of the simpler ones).

For example the basic read/write/copy operations I attempt to make use of our async stream APIs.

Note that the async variants are overall slower than the sync variants, but give the main thread/app time to "breathe" - which should make the app more usable from an end user perspective. I used setTimeout with a minimum wait threshold, so async variants will have an initial delay from the scheduling and enforcement of a minimum wait time - and that may be compounded if the implementation uses other async methods that use setTimeout to schedule.

CAN'T/WON'T DO:

chown/chmod
watch/unwatch

FOR LATER:

New promises API/FileHandle

REQUIRES SDK CHANGES TO DO:

utimes
Any link related methods (link, lstat, symlink, readLink) - though, how relevant are these in actual apps?

REQUIRES OTHER SHIMS TO DO:

createReadStream
createWriteStream

Both of these require creation of a stream shim first.

build · 2019-06-26T21:25:32Z

	Warnings
⚠️	common/Resources/ti.internal/extensions/node/buffer.js#L76 - common/Resources/ti.internal/extensions/node/buffer.js line 76 – Missing JSDoc parameter description for 'arg'. (valid-jsdoc)
⚠️	common/Resources/ti.internal/extensions/node/buffer.js#L77 - common/Resources/ti.internal/extensions/node/buffer.js line 77 – Missing JSDoc parameter description for 'encodingOrOffset'. (valid-jsdoc)
⚠️	common/Resources/ti.internal/extensions/node/buffer.js#L78 - common/Resources/ti.internal/extensions/node/buffer.js line 78 – Missing JSDoc parameter description for 'length'. (valid-jsdoc)
⚠️	tests/Resources/fs.addontest.js#L191 - tests/Resources/fs.addontest.js line 191 – The 'fs.copyFile' is not supported until Node.js 8.5.0. The configured version range is '>=8'. (node/no-unsupported-features/node-builtins)
⚠️	tests/Resources/fs.addontest.js#L218 - tests/Resources/fs.addontest.js line 218 – The 'fs.copyFileSync' is not supported until Node.js 8.5.0. The configured version range is '>=8'. (node/no-unsupported-features/node-builtins)
⚠️	tests/Resources/fs.addontest.js#L229 - tests/Resources/fs.addontest.js line 229 – 'fs.exists' was deprecated since v4.0.0. Use 'fs.stat()' or 'fs.access()' instead. (node/no-deprecated-api)
⚠️	tests/Resources/fs.addontest.js#L233 - tests/Resources/fs.addontest.js line 233 – 'fs.exists' was deprecated since v4.0.0. Use 'fs.stat()' or 'fs.access()' instead. (node/no-deprecated-api)
⚠️	tests/Resources/fs.addontest.js#L1040 - tests/Resources/fs.addontest.js line 1040 – The 'fs.copyFileSync' is not supported until Node.js 8.5.0. The configured version range is '>=8'. (node/no-unsupported-features/node-builtins)

	Messages
📖	💾 Here's the generated SDK zipfile.
📖	✅ All tests are passing Nice one! All 4403 tests are passing. (There are 479 skipped tests not included in that total)
📖	✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.

Generated by 🚫 dangerJS against 5c04349

common/Resources/ti.internal/extensions/node/fs.js

janvennemann · 2019-09-11T13:19:20Z

common/Resources/ti.internal/extensions/node/fs.js

+ * @return {Ti.Filesystem.File}
+ */
+function getTiFileFromPathLikeValue(path) {
+	// Android can't properly handle file: URIs if they don't have trailing '//'


It looks like you have actually fixed that in the Android implementation, right?

There's an open PR for it (I don't think it's been merged yet)

common/Resources/ti.internal/extensions/node/fs.js

sgtcoolguy · 2019-09-13T15:09:43Z

I pushed up come commits to address the first round of feedback...

build · 2019-09-17T18:27:43Z

tests/Resources/fs.addontest.js

+
+		it('truncates to 0 bytes by default', finished => {
+			const dest = Ti.Filesystem.tempDirectory + `truncate_${Date.now()}.js`;
+			fs.copyFileSync(thisFilePath, dest);


⚠️ tests/Resources/fs.addontest.js line 997 – The 'fs.copyFileSync' is not supported until Node.js 8.5.0. The configured version range is '>=8'. (node/no-unsupported-features/node-builtins)

common/Resources/ti.internal/extensions/node/fs.js

build · 2019-10-03T13:07:16Z

tests/Resources/fs.addontest.js

+		});
+
+		it('checks that non-existent file returns false', finished => {
+			fs.exists('/some/made/up/path', exists => {


⚠️ tests/Resources/fs.addontest.js line 244 – 'fs.exists' was deprecated since v4.0.0. Use 'fs.stat()' or 'fs.access()' instead. (node/no-deprecated-api)

build · 2019-10-03T13:07:16Z

tests/Resources/fs.addontest.js

+
+		it('truncates to 0 bytes by default', () => {
+			const dest = Ti.Filesystem.tempDirectory + `truncateSync_${Date.now()}.js`;
+			fs.copyFileSync(thisFilePath, dest);


⚠️ tests/Resources/fs.addontest.js line 1033 – The 'fs.copyFileSync' is not supported until Node.js 8.5.0. The configured version range is '>=8'. (node/no-unsupported-features/node-builtins)

janvennemann

LGTM!

sgtcoolguy · 2019-10-08T17:57:30Z

All right, need to land one of the bugfixes in this PR (as #11098) and rebase all these commits down, then I'll merge it in...

build · 2019-10-08T19:45:58Z

tests/Resources/fs.addontest.js

+
+	describe('#copyFile()', () => {
+		it('is a function', () => {
+			should(fs.copyFile).be.a.Function;


⚠️ tests/Resources/fs.addontest.js line 176 – The 'fs.copyFile' is not supported until Node.js 8.5.0. The configured version range is '>=8'. (node/no-unsupported-features/node-builtins)

build · 2019-10-08T19:45:58Z

tests/Resources/fs.addontest.js

+
+	describe('#copyFileSync()', () => {
+		it('is a function', () => {
+			should(fs.copyFileSync).be.a.Function;


⚠️ tests/Resources/fs.addontest.js line 206 – The 'fs.copyFileSync' is not supported until Node.js 8.5.0. The configured version range is '>=8'. (node/no-unsupported-features/node-builtins)

Fixes TIMOB-18583

build · 2019-10-15T14:26:21Z

tests/Resources/fs.addontest.js

+
+		it('truncates to specified number of bytes', finished => {
+			const dest = Ti.Filesystem.tempDirectory + `truncate_bytes_${Date.now()}.js`;
+			fs.copyFileSync(thisFilePath, dest);


⚠️ tests/Resources/fs.addontest.js line 1011 – The 'fs.copyFileSync' is not supported until Node.js 8.5.0. The configured version range is '>=8'. (node/no-unsupported-features/node-builtins)

sgtcoolguy added ios android feature windows work in progress 🚧 labels Jun 26, 2019

build added this to the 8.2.0 milestone Jun 26, 2019

build added the docs label Jun 27, 2019

sgtcoolguy force-pushed the node-fs-rebased branch 2 times, most recently from 67ba094 to df5b325 Compare July 1, 2019 14:57

sgtcoolguy force-pushed the node-fs-rebased branch from df5b325 to 8f94f70 Compare July 18, 2019 15:56

This was referenced Jul 31, 2019

fix(android): handle file: URIs without // after scheme #11098

Merged

fix(android): use specified Ti.Filesystem.File path to createFile() #11099

Merged

sgtcoolguy force-pushed the node-fs-rebased branch from 8f94f70 to 23d080a Compare August 7, 2019 15:54

sgtcoolguy removed the work in progress 🚧 label Aug 9, 2019

sgtcoolguy marked this pull request as ready for review August 9, 2019 15:58

sgtcoolguy requested a review from janvennemann August 9, 2019 15:58

sgtcoolguy modified the milestones: 8.2.0, 8.3.0 Sep 5, 2019

janvennemann requested changes Sep 11, 2019

View reviewed changes

sgtcoolguy requested a review from janvennemann September 13, 2019 15:09

build reviewed Sep 17, 2019

View reviewed changes

janvennemann requested changes Sep 24, 2019

View reviewed changes

common/Resources/ti.internal/extensions/node/fs.js Outdated Show resolved Hide resolved

common/Resources/ti.internal/extensions/node/fs.js Outdated Show resolved Hide resolved

common/Resources/ti.internal/extensions/node/fs.js Show resolved Hide resolved

build reviewed Oct 3, 2019

View reviewed changes

sgtcoolguy requested a review from janvennemann October 3, 2019 14:00

janvennemann approved these changes Oct 7, 2019

View reviewed changes

sgtcoolguy force-pushed the node-fs-rebased branch from 071a358 to 151b792 Compare October 8, 2019 19:30

build reviewed Oct 8, 2019

View reviewed changes

sgtcoolguy added 2 commits October 15, 2019 10:04

feat(buffer): add method to convert Buffer to Ti.Buffer

0417051

feat(fs): node compatible fs module

5c04349

Fixes TIMOB-18583

sgtcoolguy force-pushed the node-fs-rebased branch from 151b792 to 5c04349 Compare October 15, 2019 14:06

build reviewed Oct 15, 2019

View reviewed changes

sgtcoolguy merged commit 74d07c1 into tidev:master Oct 15, 2019

sgtcoolguy deleted the node-fs-rebased branch October 15, 2019 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api): add Node-compatible fs module #10998

feat(api): add Node-compatible fs module #10998

sgtcoolguy commented Jun 26, 2019 •

edited

build commented Jun 26, 2019 •

edited

janvennemann Sep 11, 2019

sgtcoolguy Sep 12, 2019

sgtcoolguy commented Sep 13, 2019

build Sep 17, 2019

build Oct 3, 2019

build Oct 3, 2019

janvennemann left a comment

sgtcoolguy commented Oct 8, 2019

build Oct 8, 2019

build Oct 8, 2019

build Oct 15, 2019

feat(api): add Node-compatible fs module #10998

feat(api): add Node-compatible fs module #10998

Conversation

sgtcoolguy commented Jun 26, 2019 • edited

build commented Jun 26, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgtcoolguy commented Sep 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janvennemann left a comment

Choose a reason for hiding this comment

sgtcoolguy commented Oct 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgtcoolguy commented Jun 26, 2019 •

edited

build commented Jun 26, 2019 •

edited