Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat!: allow to configure cookie options #106

Merged
merged 3 commits into from
Jul 10, 2023
Merged

feat!: allow to configure cookie options #106

merged 3 commits into from
Jul 10, 2023

Conversation

semiaddict
Copy link
Contributor

@semiaddict semiaddict commented Jul 3, 2023 •
edited
Loading

BREAKING CHANGE: remove stale domain configuration option
feat: add cookieOptions configuration option

📚 Description

This PR addresses the issue described in #105.

It cleans up the cookie creation code to fully use nuxt's useCookie where applicable, and allows passing options to useCookie.
Cookie options defaults to setting the path to "/" to be consistent with versions up to 5.9.2.

📝 Checklist

  • All commits follow the Conventional Commit format
  • The PR's title follows the Conventional Commit format

dargmuesli
dargmuesli requested changes Jul 3, 2023
View reviewed changes
Copy link
Owner

@dargmuesli dargmuesli left a comment

Choose a reason for hiding this comment

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

Could you amend the commit message to:

feat!: allow to configure cookie options
BREAKING CHANGE: remove `domain` configuration option, add `cookieOptions` configuration option

Or something similar. That will allow for an automatic semantic release :)

@dargmuesli
Copy link
Owner

Thank you very much for the PR! ❤️

@semiaddict
Copy link
Contributor Author

Could you amend the commit message to:

feat!: allow to configure cookie options
BREAKING CHANGE: remove `domain` configuration option, add `cookieOptions` configuration option

Or something similar. That will allow for an automatic semantic release :)

Sure, however, this PR doesn't exactly 'remove' the domain option, it just cleans it up as it wasn't being used anywhere in the code (even before my changes).

Would something like this work for you then?

feat!: allow to configure cookie options
BREAKING CHANGE: remove stale `domain` configuration option, add `cookieOptions` configuration option

BTW, what does the exclamation mark before the colon in feat!: do?

@dargmuesli
Copy link
Owner

dargmuesli commented Jul 8, 2023 •
edited
Loading

Yes, that would be fine! I just think about people maybe still having a domain set, even if it does not do anything, having build errors when that option becomes unavailable.

The exclamation mark does the same as writing "BREAKING CHANGE" to the footer

BREAKING CHANGE: a commit that has a footer BREAKING CHANGE:, or appends a ! after the type/scope, introduces a breaking API change (correlating with MAJOR in Semantic Versioning). A BREAKING CHANGE can be part of commits of any type.

https://www.conventionalcommits.org/en/v1.0.0/

I just like to use both, to make it clear on every level.

@semiaddict
feat!: allow to configure cookie options
f5b87dd 
BREAKING CHANGE: remove stale `domain` configuration option
feat: add `cookieOptions` configuration option
@semiaddict semiaddict changed the title fix: clean up cookie creation feat!: allow to configure cookie options Jul 9, 2023
@semiaddict
Copy link
Contributor Author

@dargmuesli,
Thanks for the feedback. I learned something new :)

I had to slightly modify the commit message as it didn't pass the linter.
I hope that's alright.

@dargmuesli
Copy link
Owner

Sure, that's perfectly fine!

@dargmuesli
Merge branch 'master'
37bf6ce 
Conflicts:
	src/runtime/components/CookieControl.vue
	src/runtime/methods.ts
	src/runtime/plugin.ts
@dargmuesli dargmuesli changed the base branch from master to beta July 9, 2023 17:57
@dargmuesli
Copy link
Owner

I'm merging this into the beta branch first to make a release containing all breaking changes.

@dargmuesli dargmuesli merged commit 10456f6 into dargmuesli:beta Jul 10, 2023
@dargmuesli
Copy link
Owner

Thank you very much @semiaddict! ❤️

@dargmuesli dargmuesli linked an issue Jul 10, 2023 that may be closed by this pull request
Consent is now being requested on a per path basis, which wasn't the case in v5.9.2 #105
Closed
dargmuesli added a commit that referenced this pull request Jul 12, 2023
@dargmuesli @renovate @semiaddict
feat!: release v6 (#81)
feb598a 
* feat(iframe)!: remove `initialState`

BREAKING CHANGE: Setting an initial state for a cookie (i.e. enabled instead of disabled) was never properly implemented. To avoid confusion, this setting is removed. It continues to be possible to set `isIframeBlocked` true, though as of 2022-03-30 it is not yet implemented for vite (#79).

* chore(deps): update dependency typescript to v5.0.3

* chore(deps): update dependency @nuxt/kit to v3.3.3

* chore(deps): update dependency nuxt to v3.3.3

* chore(deps): update dargmuesli/github-actions action to v0.35.6

* chore(deps): update dependency webpack to v5.78.0

* chore(deps): update dependency lint-staged to v13.2.1

* chore(deps): update dependency typescript to v5.0.4

* chore(deps): update dependency eslint to v8.38.0

* chore(deps): update dependency @nuxt/kit to v3.4.0

* chore(deps): update dargmuesli/github-actions action to v0.35.7

* chore(deps): update dependency @nuxt/module-builder to v0.3.0

* chore(deps): update dependency webpack to v5.79.0

* chore(deps): update dependency @nuxt/kit to v3.4.1

* chore(deps): update dependency nuxt to v3.4.1

* chore(deps): lock file maintenance

* chore(deps): update dargmuesli/github-actions action to v0.35.8

* chore(deps): update dependency webpack to v5.80.0

* chore(deps): update dependency @nuxt/kit to v3.4.2

* chore(deps): update dependency nuxt to v3.4.2

* chore(deps): update dependency vue-tsc to v1.4.1

* chore(deps): update dependency vue-tsc to v1.4.2

* chore(deps): update dependency js-cookie to v3.0.4

* chore(deps): update dependency eslint to v8.39.0

* chore(deps): update dependency vue-tsc to v1.4.3

* chore(deps): update dependency vue-tsc to v1.4.4

* chore(deps): update dependency prettier to v2.8.8

* chore(deps): update dargmuesli/github-actions action to v0.36.0

* chore(deps): update dependency js-cookie to v3.0.5

* chore(deps): update dargmuesli/github-actions action to v0.37.2

* chore(deps): update dargmuesli/github-actions action to v0.37.3

* chore(deps): update dependency lint-staged to v13.2.2

* chore(deps): update dependency webpack to v5.81.0

* chore(deps): update dependency vue-tsc to v1.6.0

* chore(deps): update dependency vue-tsc to v1.6.1

* chore(deps): update dependency @nuxt/kit to v3.4.3

* chore(deps): update dependency nuxt to v3.4.3

* chore(deps): lock file maintenance

* chore(deps): update dependency vue-tsc to v1.6.2

* chore(deps): update dependency @nuxt/module-builder to v0.3.1

* chore(deps): update dependency vue-tsc to v1.6.3

* chore(deps): update dargmuesli/github-actions action to v0.37.4

* chore(deps): update dependency webpack to v5.82.0

* chore(deps): update dependency vue-tsc to v1.6.4

* chore(deps): update dependency eslint to v8.40.0

* chore(deps): update pnpm to v8.5.0

* chore(deps): update dargmuesli/github-actions action to v0.38.1

* chore(deps): update dependency webpack to v5.82.1

* chore(deps): update dependency vue to v3.3.1 (#100)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore(deps): update dependency vue to v3.3.2

* chore(deps): update dargmuesli/github-actions action to v0.38.2

* chore(deps): update dependency vue-tsc to v1.6.5

* chore(deps): update dargmuesli/github-actions action to v0.40.3

* chore(deps): lock file maintenance

* chore(deps): update dargmuesli/github-actions action to v0.40.4

* chore(deps): update pnpm to v8.5.1

* chore(deps): update dependency @nuxt/kit to v3.5.0

* chore(deps): update dependency nuxt to v3.5.0

* chore(deps): update dargmuesli/github-actions action to v0.40.5

* chore(deps): update dependency @sindresorhus/slugify to v2.2.1

* chore(deps): update dependency webpack to v5.83.1

* chore(deps): update dependency vue to v3.3.4

* chore(deps): update dependency eslint to v8.41.0

* feat!: allow to configure cookie options (#106)

BREAKING CHANGE: remove stale `domain` configuration option
feat: add `cookieOptions` configuration option

Co-authored-by: Jonas Thelemann <[email protected]>

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Oussama Mubarak <[email protected]>
dargmuesli added a commit that referenced this pull request Jul 12, 2023
@semantic-release-bot @dargmuesli
chore(release): 6.0.0 [skip ci]
f87382e 
# [6.0.0](5.10.4...6.0.0) (2023-07-12)

* feat!: release v6 (#81) ([feb598a](feb598a)), closes [#81](#81) [#100](#100) [#106](#106)

### BREAKING CHANGES

* Setting an initial state for a cookie (i.e. enabled instead of disabled) was never properly implemented. To avoid confusion, this setting is removed. It continues to be possible to set `isIframeBlocked` true, though as of 2022-03-30 it is not yet implemented for vite (#79).

* chore(deps): update dependency typescript to v5.0.3

* chore(deps): update dependency @nuxt/kit to v3.3.3

* chore(deps): update dependency nuxt to v3.3.3

* chore(deps): update dargmuesli/github-actions action to v0.35.6

* chore(deps): update dependency webpack to v5.78.0

* chore(deps): update dependency lint-staged to v13.2.1

* chore(deps): update dependency typescript to v5.0.4

* chore(deps): update dependency eslint to v8.38.0

* chore(deps): update dependency @nuxt/kit to v3.4.0

* chore(deps): update dargmuesli/github-actions action to v0.35.7

* chore(deps): update dependency @nuxt/module-builder to v0.3.0

* chore(deps): update dependency webpack to v5.79.0

* chore(deps): update dependency @nuxt/kit to v3.4.1

* chore(deps): update dependency nuxt to v3.4.1

* chore(deps): lock file maintenance

* chore(deps): update dargmuesli/github-actions action to v0.35.8

* chore(deps): update dependency webpack to v5.80.0

* chore(deps): update dependency @nuxt/kit to v3.4.2

* chore(deps): update dependency nuxt to v3.4.2

* chore(deps): update dependency vue-tsc to v1.4.1

* chore(deps): update dependency vue-tsc to v1.4.2

* chore(deps): update dependency js-cookie to v3.0.4

* chore(deps): update dependency eslint to v8.39.0

* chore(deps): update dependency vue-tsc to v1.4.3

* chore(deps): update dependency vue-tsc to v1.4.4

* chore(deps): update dependency prettier to v2.8.8

* chore(deps): update dargmuesli/github-actions action to v0.36.0

* chore(deps): update dependency js-cookie to v3.0.5

* chore(deps): update dargmuesli/github-actions action to v0.37.2

* chore(deps): update dargmuesli/github-actions action to v0.37.3

* chore(deps): update dependency lint-staged to v13.2.2

* chore(deps): update dependency webpack to v5.81.0

* chore(deps): update dependency vue-tsc to v1.6.0

* chore(deps): update dependency vue-tsc to v1.6.1

* chore(deps): update dependency @nuxt/kit to v3.4.3

* chore(deps): update dependency nuxt to v3.4.3

* chore(deps): lock file maintenance

* chore(deps): update dependency vue-tsc to v1.6.2

* chore(deps): update dependency @nuxt/module-builder to v0.3.1

* chore(deps): update dependency vue-tsc to v1.6.3

* chore(deps): update dargmuesli/github-actions action to v0.37.4

* chore(deps): update dependency webpack to v5.82.0

* chore(deps): update dependency vue-tsc to v1.6.4

* chore(deps): update dependency eslint to v8.40.0

* chore(deps): update pnpm to v8.5.0

* chore(deps): update dargmuesli/github-actions action to v0.38.1

* chore(deps): update dependency webpack to v5.82.1
* remove stale `domain` configuration option
feat: add `cookieOptions` configuration option

Co-authored-by: Jonas Thelemann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dargmuesli dargmuesli dargmuesli approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Consent is now being requested on a per path basis, which wasn't the case in v5.9.2
2 participants
@semiaddict @dargmuesli