Fixed RegExp in filter-adapter.ts and sort-context.ts to work in Safari

[papaIOprog]

Pull Request

Related issue

Fixes #1277

What does this PR do?

• Replaces regular expressions that use lookbehind.
• Resolves a bug specifically affecting Safari.

[changeset-bot]

🦋 Changeset detected

Latest commit: aeb1b4a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@meilisearch/instant-meilisearch	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[curquiza]

Hello @papaIOprog
Thanks for the PR
Since your PR impacts the library, can you follow the step in the CONTRIBUTING.md regarding changeset?
https://github.com/meilisearch/meilisearch-js-plugins/blob/main/CONTRIBUTING.md#versioning-with-changesets

Also, can you ensure the PR passes the tests? I see some tests in the CI failing.

[curquiza]

Also, can you add tests please? 😊

[papaIOprog]

I think I created the changeset incorrectly. I'm going to fix this.

[papaIOprog]

Also, can you add tests please? 😊

Did you mean to add tests for these methods?

[curquiza]

Did you mean to add tests for these methods?

Yes please, sorry maybe we already have and you did not have to change them
I'm not an expert in this repo, trying to do my best with the review 😊

[papaIOprog]

Did you mean to add tests for these methods?

Yes please, sorry maybe we already have and you did not have to change them I'm not an expert in this repo, trying to do my best with the review 😊

I checked the repository and there are tests for these methods, but I'll add one corner case soon.

[papaIOprog]

Here are links to tests that already exist.

For splitSortString:

• split multiple sorting rules
• split one sorting rule
• split no sorting rule

For splitNumericFilter:

• split multiple numeric filters
• split equal filter
• split not equal filter

[papaIOprog]

@curquiza I added 1 test that checks the case when an unusual sorting rule (_geoPoint, for example) is not in the first place in the list.

[papaIOprog]

@curquiza Hi! I just want to check if everything is ok. Do I need to do anything else?

[curquiza]

Sorry @papaIOprog for the delay, working on a lot of things at the same time

@flevi29 if you are around, I would love to have your opinion on this one before reviewing or merging 🙏

@brunoocasali and I (meili maintainers for all the integrations like this one) are not JS expert at all

[flevi29]

I'm pretty sure Babel should be transforming this stuff. Babel is misconfigured.

meilisearch-js-plugins/packages/instant-meilisearch/.babelrc

Lines 1 to 12 in 4d9991e

	{
	"presets": [
	[
	"@babel/preset-env",
	{
	"targets": {
	"node": "current"
	}
	}
	]
	]
	}

It's targeting ... just node? Even at that current? That's at minimum ES2022. In my opinion it should be targeting the last 2 versions of the more popular browsers (Chrome, Firefox, Edge, Safari) (but my opinion might not be the most educated, see browserlists), but since meilisearch-js at the current moment targets IE11, let's just do that. It looks like this:
https://github.com/meilisearch/meilisearch-js/blob/177947ddaf5b9363152cb3a139b0d4572d7b5ab6/rollup.config.js#L46-L60

@papaIOprog Can you please fiddle with Babel, maybe even update the version that's used, and see if that transforms the RegExp properly? We shouldn't write code for older browsers, we should transform/transpile it for them.

This is not to say that the other changes are bad, I might've not considered every possible combination when I wrote that.

[papaIOprog]

@flevi29 Hello! I can test this, but I found that Babel members said that it cannot be polyfilled.
babel/babel#11086 (comment)
babel/babel#10699 (comment)

[flevi29]

Oh, okay, that's unfortunate. Then I don't have any objections to this PR.

[flevi29]

It would be nice if Babel at least could warn you about unsupported features, but I can't even find any real discussion about it.
https://stackoverflow.com/questions/76135819/babeljs-does-not-warn-for-unsupported-regex-syntax-with-older-safari-version

We'll just have to wait for people to report these issues :\ .

[curquiza]

thanks @papaIOprog and @flevi29 for your work on this! Really apprecidated

bors merge

[meili-bors]

Build succeeded:

• autocomplete-client end-to-end-tests
• instant-meilisearch end-to-end-tests
• integration-tests (Node.js 14)
• integration-tests (Node.js 16)
• integration-tests (Node.js 18)
• style-check
• types-check