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

Fix output for es5 #408

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

Merged
armandabric merged 3 commits into algolia:master from Chris-Baker:fix-output-for-es5
Sep 15, 2019
Merged

Conversation

Copy link
Contributor

@Chris-Baker Chris-Baker commented Sep 2, 2019

Fix for #387 to replace stringify-object and make the output compatible with ES5.

stevethatcodes, alexgmason, and tu4mo reacted with thumbs up emoji
Copy link
Contributor

vvo commented Sep 3, 2019

@Spy-Seth this seems good for me, tests are passing 🤷‍♂ thoughts?

Copy link
Collaborator

armandabric commented Sep 3, 2019
edited
Loading

I'm not fan to use a community fork of stringify-object only dedicated to this package. If we use a fork we should manage it (as there is no strong alternative package). @vvo Could we fork it and publish it from the Algolia org?

Copy link
Collaborator

armandabric commented Sep 4, 2019
edited
Loading

To clarify my answer from yesterday, I'm all in favor of resolving the stringify-object bundling issue. It's a pain in the ass for us for too long: source of bugs in the browser and maintenance headaches (we are stuck in the rollup upgrade because of it...)

Copy link
Contributor Author

Hi @Spy-Seth, @vvo , let me know if you want me to make changes to this once you have decided whether to publish an internal fork.

armandabric reacted with thumbs up emoji

Copy link
Contributor

vvo commented Sep 9, 2019

@vvo Could we fork it and publish it from the Algolia org?
I would rather use the one from @Chris-Baker since it seems he had the issue and he would be the best to maintain it. There's also https://github.com/searls/stringify-object/tree/transform-es5 which already existed too if we want to reuse something and if @Chris-Baker want to work with @searls.

Latest release from stringify-object was one year ago, I believe it's fine to just merge this if the only issue is about who owns the fork. Or reuse https://github.com/searls/stringify-object/tree/transform-es5.

@Spy-Seth I won't be at Algolia forever and don't want Algolia to own something they did not need (stringify object es5) but rather trust the community (@Chris-Baker) to do good.

Copy link
Collaborator

@armandabric armandabric left a comment

Choose a reason for hiding this comment

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

Everything is 👌 for me.

I just remove some stringify-object rollup configuration and rebase the PR on master.

Let's merge this. Thanks a lot for the work @Chris-Baker

@armandabric armandabric merged commit b636836 into algolia:master Sep 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@armandabric armandabric armandabric approved these changes

@vvo vvo Awaiting requested review from vvo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

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