Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Updated "isemail" package to ver. 3.2.x#4241

Merged
bajtos merged 1 commit into
strongloop:master from
Sarbinski:issue-4239/update-ismail-lib-for-mail-validation
Oct 7, 2019
Merged

Updated "isemail" package to ver. 3.2.x #4241
bajtos merged 1 commit into
strongloop:master from
Sarbinski:issue-4239/update-ismail-lib-for-mail-validation

Conversation

@Sarbinski

@Sarbinski Sarbinski commented Aug 9, 2019
edited by bajtos
Loading

Copy link
Copy Markdown

Description

isemail version 3.x.x is completely rewritten and not depend on NodeJS native modules.

Resolve #4239

slnode commented Aug 9, 2019

Copy link
Copy Markdown

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@bajtos bajtos self-assigned this Aug 22, 2019

bajtos commented Aug 22, 2019

Copy link
Copy Markdown
Member

Thank you for the pull request. It looks is v3 of isemail does not work in the browser out of the box and thus it's breaking out tests verifying LoopBack functionality in browser (see the failed CI builds on Travis CI). In order to land this PR, we need to find a way how to make isemail work. It may be enough to tell browserify to ignore certain dependencies when creating the browser bundle, see the current config:

loopback/package.json

Lines 109 to 117 in 9b1d488

"browser": {
"express": "./lib/browser-express.js",
"./lib/server-app.js": "./lib/browser-express.js",
"connect": false,
"nodemailer": false,
"supertest": false,
"depd": "loopback-datasource-juggler/lib/browser.depd.js",
"bcrypt": false
},

bajtos commented Aug 23, 2019

Copy link
Copy Markdown
Member

On the second thought, it's also possible that the new version of isemail is using modern JS constructs from ES6+ that are not supported by PhantomJS that we use to run our tests in the browser. This could be solved e.g. by adding Babel transpiler to our test setup or by switching from PhantomJS to Chrome Headless (or Firefox Headless). Either way, such change should be ideally made in a new pull request, or at least in a standalone commit clearly separated from the is-email update.

bajtos commented Sep 12, 2019

Copy link
Copy Markdown
Member

The Karma test failures may be unrelated to the changes made in this pull request, see #4252

bajtos commented Sep 12, 2019

Copy link
Copy Markdown
Member

I checked out this feature branch and run the tests locally, unfortunately they are failing with the new isemail version (while passing with the older version).

PhantomJS 2.1.1 (Mac OS X 0.0.0) ERROR
 TypeError: undefined is not a constructor (evaluating ''0円'.normalize('NFC')')
 at /var/folders/ht/v2sc3fbn5m77w03kybtbf6h40000gp/T/0d4a49f78735968e521f5df205d36d0c.browserify.js:67609:48
PhantomJS 2.1.1 (Mac OS X 0.0.0): Executed 0 of 0 ERROR (0.404 secs / 0 secs)
PhantomJS 2.1.1 (Mac OS X 0.0.0) ERROR
 TypeError: undefined is not an object (evaluating 'ACL.WRITE')
 at /var/folders/ht/v2sc3fbn5m77w03kybtbf6h40000gp/T/0d4a49f78735968e521f5df205d36d0c.browserify.js:174236:31
PhantomJS 2.1.1 (Mac OS X 0.0.0): Executed 0 of 0 ERROR (0.406 secs / 0 secs)
PhantomJS 2.1.1 (Mac OS X 0.0.0) ERROR
 TypeError: undefined is not an object (evaluating 'loopback.Checkpoint.extend')
 at /var/folders/ht/v2sc3fbn5m77w03kybtbf6h40000gp/T/0d4a49f78735968e521f5df205d36d0c.browserify.js:172675:37
PhantomJS 2.1.1 (Mac OS X 0.0.0): Executed 0 of 0 ERROR (0.407 secs / 0 secs)
PhantomJS 2.1.1 (Mac OS X 0.0.0) ERROR
 TypeError: Object is not a constructor (evaluating 'it')
 at /var/folders/ht/v2sc3fbn5m77w03kybtbf6h40000gp/T/0d4a49f78735968e521f5df205d36d0c.browserify.js:170898:7
PhantomJS 2.1.1 (Mac OS X 0.0.0): Executed 0 of 0 ERROR (0.407 secs / 0 secs)

@bajtos bajtos force-pushed the issue-4239/update-ismail-lib-for-mail-validation branch from 8b66b30 to 7094a3c Compare October 7, 2019 09:18

bajtos commented Oct 7, 2019

Copy link
Copy Markdown
Member

Rebased on top of the latest master to apply the CI fix from #4262.

@bajtos bajtos force-pushed the issue-4239/update-ismail-lib-for-mail-validation branch from 7094a3c to 4846048 Compare October 7, 2019 09:19
@bajtos bajtos merged commit da51c99 into strongloop:master Oct 7, 2019

bajtos commented Oct 7, 2019

Copy link
Copy Markdown
Member

Landed, thank you for the contribution! ❤️

Sarbinski reacted with thumbs up emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clark0x clark0x Awaiting requested review from clark0x

@ebarault ebarault Awaiting requested review from ebarault ebarault is a code owner

@fabien fabien Awaiting requested review from fabien fabien is a code owner

@zbarbuto zbarbuto Awaiting requested review from zbarbuto zbarbuto is a code owner

1 more reviewer

@bajtos bajtos bajtos approved these changes

Reviewers whose approvals may not affect merge requirements

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Update isemail dependency

AltStyle によって変換されたページ (->オリジナル) /