Remove underscore #331

harryadel · 2021-04-27T17:42:31Z

Please bear with me as this my first time contributing to the Blaze project.

The goal of this pull request is to:

Remove underscore and all of its methods in the code
~~Modernize the code base~~

Both test:ci and test:watch yield no errors.

That's for the code part but the site Hexo server isn't working. I noticed it reads jsdoc comments from several files like builtins.js, preamble.js and couple more so I reverted the linting on those files but then a weird error showed up in the browser

Uncaught SyntaxError: Invalid regular expression: /(?:[ Ã‚â€º][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-PRZcf-nqry=><])|[

I tried reverting the linting on the whole project and even updating the dependencies to the latest in attempt at least to get a more readable error but in vain.

I attempted to remove the underscore dependency from site directory but then reverted my changes so you guys can have easier time debugging things.

StorytellerCZ · 2021-04-28T13:59:51Z

That is a big PR. A lot of it is just change in codying style, which needlessly bloats this PR. Could you maybe separate those into its own PR?

harryadel · 2021-04-28T14:01:13Z

So split the code modernization into its own PR?

harryadel · 2021-04-28T14:40:54Z

@StorytellerCZ Done!

harryadel · 2021-04-28T14:49:12Z

I'll open up the modernization PR once this one gets merged. 👍

StorytellerCZ

Looks good to me. I'm thinking that if we can combine this together with removal of jQuery and general modernization that you proposed, then we can do major version bump.

harryadel · 2021-04-30T16:17:30Z

Looks good to me. I'm thinking that if we can combine this together with removal of jQuery and general modernization that you proposed, then we can do major version bump.

The removal of jQuery would definitely require a major version bump because as @jankapunkt pointed out people's Blaze code is making heavy use of internal jQuery method.

This requires very precise planning to ensure the API is not breaking (think of templateInstance.$(...))

filipenevola

Hi @harryadel , thank you for the work here.

For the review to be effective we need to have PRs that are doing only one thing.

Could you keep only the underscore changes in this PR so we can move forward with it?

harryadel · 2021-05-05T14:45:57Z

@filipenevola There's nothing but underscore changes. I've removed the modernization commits as Jan recommended.

StorytellerCZ · 2021-05-05T16:45:23Z

@harryadel I think the issue there is that your IDE or something automatic also "fixed" lot of the code around.

packages/templating-runtime/templating.js

jankapunkt · 2021-05-05T16:49:29Z

As far as I saw, that was mostly lint fixes which is why I thought them to be ok

harryadel · 2021-05-05T18:17:58Z

I see so the problem lies with the extra linting. Ok, will remove it and notify you guys.

harryadel · 2021-05-05T19:55:36Z

@filipenevola I guess it's ready now

filipenevola

See my comments ;)

filipenevola · 2021-05-10T17:22:19Z

packages/blaze/builtins.js

@@ -37,8 +37,9 @@ Blaze.With = function (data, contentFunc) {
 */
 Blaze._attachBindingsToView = function (bindings, view) {
  view.onViewCreated(function () {
-    _.each(bindings, function (binding, name) {
-      view._scopeBindings[name] = new ReactiveVar;
+    Object.keys(bindings).forEach(function (name) {


Object.entries is better in this case

filipenevola · 2021-05-10T17:24:59Z

packages/blaze/builtins.js


-      eachView.argVar.set(arg);


is this change replacing underscore?

filipenevola · 2021-05-10T17:27:44Z

packages/blaze/util.js

@@ -0,0 +1,25 @@
+has = function (obj, key) {


we shouldn't expose anything globally, use exports / imports instead.

maybe it would be better to use specific modules of lodash instead of reimplementing these functions here.

harryadel · 2021-05-12T10:54:15Z

@filipenevola friendly ping, please let me know what you think.

filipenevola

There are still places where you should use a different Object utility and also some places where you should use lodash.

packages/blaze/template.js

packages/blaze/view.js

packages/caching-html-compiler/caching-html-compiler.js

packages/spacebars-compiler/codegen.js

packages/templating-runtime/dynamic.js

packages/templating-tools/compile-tags-with-spacebars.js

harryadel · 2021-05-13T21:11:18Z

So the problem that was causing the site to fail was the non existence of files within the themes directory which is fixed by git submodule update --init --recursive. Should've cloned recursively as suggested in the constributing.md. 😅

But there's a tiny problem with installing modules for the site though which I'll fix in a different PR.

harryadel · 2021-05-28T17:01:08Z

@filipenevola @StorytellerCZ

filipenevola

❤️

Remove underscore

8ee7f0f

Revert changes made to site

c032c01

harryadel force-pushed the remove-underscore branch from f2e9444 to c032c01 Compare April 28, 2021 14:40

StorytellerCZ approved these changes Apr 30, 2021

View reviewed changes

filipenevola requested changes May 5, 2021

View reviewed changes

StorytellerCZ requested changes May 5, 2021

View reviewed changes

packages/templating-runtime/templating.js Show resolved Hide resolved

harryadel added 2 commits May 5, 2021 20:14

Revert lint changes

4a05e73

Merge branch 'master' of github.com:meteor/blaze into remove-underscore

c485706

harryadel added 3 commits May 5, 2021 21:15

Fix failing tests

cd6ca38

More delinting

85669dc

Fix failing test

47849a6

harryadel requested review from filipenevola and StorytellerCZ May 5, 2021 19:55

StorytellerCZ approved these changes May 6, 2021

View reviewed changes

filipenevola requested changes May 10, 2021

View reviewed changes

harryadel added 3 commits May 10, 2021 20:56

Use Object.entries

40bc76b

Use lodash NPM methods instead of utils methods

a476bb6

Revert lint changes in eachView.onViewCreated

2adc56e

harryadel requested a review from filipenevola May 10, 2021 19:49

StorytellerCZ approved these changes May 12, 2021

View reviewed changes

filipenevola requested changes May 12, 2021

View reviewed changes

harryadel added 5 commits May 13, 2021 05:08

Use Object.values

c510955

Import lodash isEmpty in caching compiler

fc89a83

Use includes instead of index

19d332e

Addlodash.has and ecmascript packages in templating runtime

c765aa4

Use lodash.isempty in templating-tools

75fd80d

harryadel requested review from filipenevola and StorytellerCZ May 13, 2021 03:27

StorytellerCZ approved these changes May 30, 2021

View reviewed changes

filipenevola approved these changes Jun 4, 2021

View reviewed changes

StorytellerCZ merged commit bd64eab into meteor:master Jun 4, 2021

brucejo75 mentioned this pull request Jun 13, 2021

Blaze repo issues #340

Closed

harryadel mentioned this pull request Sep 9, 2021

relplace lodash with underscore Meteor-Community-Packages/meteor-collection2#429

Closed

harryadel mentioned this pull request Jan 23, 2022

Remove underscore meteor/meteor#11869

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove underscore #331

Remove underscore #331

harryadel commented Apr 27, 2021 •

edited

Loading

StorytellerCZ commented Apr 28, 2021

harryadel commented Apr 28, 2021

harryadel commented Apr 28, 2021

harryadel commented Apr 28, 2021

StorytellerCZ left a comment

harryadel commented Apr 30, 2021 •

edited

Loading

filipenevola left a comment

harryadel commented May 5, 2021

StorytellerCZ commented May 5, 2021

jankapunkt commented May 5, 2021

harryadel commented May 5, 2021

harryadel commented May 5, 2021

filipenevola left a comment

filipenevola May 10, 2021

filipenevola May 10, 2021

filipenevola May 10, 2021

harryadel commented May 12, 2021

filipenevola left a comment

harryadel commented May 13, 2021 •

edited

Loading

harryadel commented May 28, 2021

filipenevola left a comment

Remove underscore #331

Remove underscore #331

Conversation

harryadel commented Apr 27, 2021 • edited Loading

StorytellerCZ commented Apr 28, 2021

harryadel commented Apr 28, 2021

harryadel commented Apr 28, 2021

harryadel commented Apr 28, 2021

StorytellerCZ left a comment

Choose a reason for hiding this comment

harryadel commented Apr 30, 2021 • edited Loading

filipenevola left a comment

Choose a reason for hiding this comment

harryadel commented May 5, 2021

StorytellerCZ commented May 5, 2021

jankapunkt commented May 5, 2021

harryadel commented May 5, 2021

harryadel commented May 5, 2021

filipenevola left a comment

Choose a reason for hiding this comment

filipenevola May 10, 2021

Choose a reason for hiding this comment

filipenevola May 10, 2021

Choose a reason for hiding this comment

filipenevola May 10, 2021

Choose a reason for hiding this comment

harryadel commented May 12, 2021

filipenevola left a comment

Choose a reason for hiding this comment

harryadel commented May 13, 2021 • edited Loading

harryadel commented May 28, 2021

filipenevola left a comment

Choose a reason for hiding this comment

harryadel commented Apr 27, 2021 •

edited

Loading

harryadel commented Apr 30, 2021 •

edited

Loading

harryadel commented May 13, 2021 •

edited

Loading