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

Add SimplenoteImporter #975

Merged
merged 3 commits into from
Nov 2, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add SimplenoteImporter
  • Loading branch information
mirka committed Nov 2, 2018
commit 03d4f1cd52a3819094fd4823de1ec25418543e72
6 changes: 3 additions & 3 deletions lib/utils/import/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class CoreImporter extends EventEmitter {
this.noteBucket.add(importedNote);
};

importNotes = (notes = {}) => {
importNotes = (notes = {}, options) => {
if (isEmpty(notes)) {
this.emit('status', 'error', 'No notes to import.');
return;
Expand All @@ -80,9 +80,9 @@ class CoreImporter extends EventEmitter {
return;
}

get(notes, 'activeNotes', []).map(note => this.importNote(note));
get(notes, 'activeNotes', []).map(note => this.importNote(note, options));
get(notes, 'trashedNotes', []).map(note =>
this.importNote(note, { isTrashed: true })
this.importNote(note, { ...options, isTrashed: true })
);
};
}
Expand Down
59 changes: 59 additions & 0 deletions lib/utils/import/simplenote/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { EventEmitter } from 'events';
import CoreImporter from '../';
import { endsWith, isEmpty } from 'lodash';

class SimplenoteImporter extends EventEmitter {
constructor({ noteBucket, tagBucket, options }) {
super();
this.noteBucket = noteBucket;
this.tagBucket = tagBucket;
this.options = options;
}

importNotes = filesArray => {
const coreImporter = new CoreImporter({
noteBucket: this.noteBucket,
tagBucket: this.tagBucket,
});

if (isEmpty(filesArray)) {
this.emit('status', 'error', 'No file to import.');
return;
}

const file = filesArray[0];

if (!endsWith(file.name.toLowerCase(), '.json')) {
this.emit('status', 'error', 'File name does not end in ".json".');
return;
}

// Limit file size we will read to 2mb
if (file.size > 2000000) {
Copy link
Contributor

@roundhill roundhill Oct 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a Simplenote export file with 1,600 notes that is just over 2mb. Maybe we should set a 5mb limit? Not sure if you tested it with a large export or not.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today I tested with export files of varying sizes (80KB – 4MB). While large files did not choke the app in terms of memory, it did degrade the experience in non-trivial ways:

  1. There is considerable lag between the point when all notes have been added to the noteBucket, and the point when the notes appear (all at once) in the Note List.
  2. Clicking on notes in the Note List is unresponsive for a while. The onClick is firing, but the redux dispatch is being deferred until a certain point... I'm not immediately sure what is causing this.
  3. It takes quite a while for all the notes to be synced over to the server, and it is almost impossible to tell when it has finished. The only way to tell is to try clicking the Log Out button and see if it throws an unsynced notes warning.
  4. If there are lots of notes AND lots of tags, the tags take a long time to show up in the tag drawer.

With these constraints, I felt like a 500-note (~400KB) export file was as far as I wanted to go... 😵 And since these issues are occurring after the CoreImporter is done, I'm guessing the other importers will encounter them too.

I'll see if there are some easy things we can do to address at least 1 and 2.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ Addressed in #986 #987 #988

this.emit('status', 'error', 'File should be less than 2 MB.');
return;
}

const fileReader = new FileReader();

fileReader.onload = event => {
const fileContent = event.target.result;

if (!fileContent) {
this.emit('status', 'error', 'File was empty.');
return;
}

const dataObj = JSON.parse(fileContent);
const noteCount =
dataObj.activeNotes.length + dataObj.trashedNotes.length;

coreImporter.importNotes(dataObj, this.options);
this.emit('status', 'complete', noteCount);
};

fileReader.readAsText(file);
};
}

export default SimplenoteImporter;