No more path syntax

[ThePrimeagen]

The removal of falcor-path-syntax from falcor library. Equates to ~10k file size reduction.

[tivac]

Is there a discussion of this somewhere? Is it all arrays all the time in falcor unless you choose to load falcor-path-syntax now?

I don't mind, I just want to understand.

[ThePrimeagen]

@sdesai will you have time to review? @tivac we had this conversation internally over the last 6 months and looks like we simply forgot to create an issue. The thing about path-syntax is that its convenient for getting started with falcor and for presentations. For practical use of path-syntax makes most sense is in es6 template strings. Since this is the case, it does not make much sense to include a sizable amount of code for a use case that can be included via require. As for performance, using path-syntax is not performant. It has to convert the string into an array then the rest of the algorithm runs as normal.

[tivac]

@michaelbpaulson Makes sense, I haven't used the path-syntax in actual code yet.

[sdesai]

Yes, I can review.

We should still broadcast this change though, through an issue at the very least, so that other users (who may be using path syntax) have visibility around what they need to change if they need to continue using path syntax as an external module, and, the ultimate ES6/transpiled direction around ES6 tagged template strings. e.g.

path`genreList.0.0.item` 

[jameswomack]

Nice work! Quick input regarding the relationship between these changes and the version number:
As crazy as it sounds, removing falcor-path-syntax can be a breaking change. I've maintained modules for a while and some module consumers write code that digs into implementation details like this. Since the cost of doing so is next-to-nothing, I usually side w/ caution and up the big number. If confident that it won't break anyone's code or I have control over the consuming parties, I'll up the path version.

[ThePrimeagen]

@jameswomack we are going to be upping the major version with our next release as we have changed the deref interface and removed path syntax. I am hoping to get rid of Rx dependency here shortly, but if not, then we will have to up it again.

[jameswomack]

Got it @michaelbpaulson, good to know 😸

[sdesai]

We could collapse these now.

[sdesai]

Reviewed. A couple of comments inline.

NOTES:

• We should also update the docs before merging this in. Otherwise it's misleading for folks working off of master. I'll do this, if someone doesn't get to first.
• You'll need to build the falcor.all dist versions and commit, before merging [ dist/falcor.all.js, dist/falcor.all.min.js are deleted with this PR ]. I'll set aside some time to actually build these artifacts as part of Travis CI, but for now, you should commit them.
• I'm still a little concerned about not having a way to broadcast this more effectively. I think we should add a section to our documentation pages/README.md to collate back-compat breaking changes (deref + path syntax currently) and bump up semver of course, when we publish next.

[tivac]

@sdesai Two things.

1. Hi! Feels like a long time since YUI :-\
2. 👍 x 💯 to a collation of back-compat breaking changes. The semver rev is an obvious clue but calling it out specifically is hugely valuable.

[sdesai]

Hey! Small World!

[ThePrimeagen]

1. @sdesai its about 25,000 miles around the globe, so it is small compared with Jupiter.
2. I would argue that the documentation update should be done when we cut a new version of Falcor. Since if I npm i as of today, path syntax is still available.
3. This is where a CHANGELOG file would be nice so we can put down notes. Even better would be to develop a way to automagically create these updates.

[sdesai]

I would argue that the documentation update should be done when we cut a new version of Falcor. Since if I npm i as of today, path syntax is still available.

That's a valid point, but I think updating the docs means:

1. That the docs work for both npm i AND people working off master (what's on NPM works fine with the array syntax). If we don't update the docs, we'd need a caveat on the docs, which specifies the exception for folks working off master.
2. The longer the docs have path syntax, the more path syntax will be used by end users, and need to be migrated.

The motivation for both of these is to minimize end-user confusion. Let me know if that changes your mind. I'm happy to do the updating (well not happy, but happy), just may be a couple of days.

This is where a CHANGELOG file would be nice so we can put down notes.

Let's start with a CHANGELOG file, and figure out if there's a syntax we want to use in commits to filter out "relavent" change log commit messages, or just drive it off issues closed by a commit range (or something else).

[trxcllnt]

This is where a CHANGELOG file would be nice so we can put down notes. Even better would be to develop a way to automagically create these updates.

@sdesai @michaelbpaulson With the help of @jeffbcross, ReactiveX/RxJS generates its change log from the commit messages, Angular-style.

[sdesai]

Thanks. That may be a good place to start. I'll take a look.

[ThePrimeagen]

@sdesai could you list what you would like for me to do to merge this branch in?

[sdesai]

I'd like us to decide what we want to do for documentation/exposing this change more broadly. I've stated my opinion above, and was waiting for your feedback.

To clarify where I'm coming from - I think it's more than just getting the code in. I'd like us to consider the impact to the end users and present the right migration support.

[ThePrimeagen]

This will be on hold for a moment as it forces us to address some core library requirements. I am not going to continue with this until we figure out the documentation updates, readme updates, and changelog integration.

[steveorsomethin]

This is still on hold, likely for a 3.x.

[asyncanup]

Closing, will be available for historical context, doesn't need to remain open.