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

fix: webpack bundling compatibility #757

Merged
merged 1 commit into from
Jan 18, 2024
Merged

fix: webpack bundling compatibility #757

merged 1 commit into from
Jan 18, 2024

Conversation

cbazureau
Copy link

fix #755

@cbazureau cbazureau mentioned this pull request Jan 18, 2024
Closed
lorenwest
lorenwest approved these changes Jan 18, 2024
View reviewed changes
Copy link
Collaborator

@lorenwest lorenwest left a comment

Choose a reason for hiding this comment

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

Looks good.

parser.js
// webpack resolves json5 with module field out of the box which lead to this usage
// @see https://github.com/node-config/node-config/issues/755
// @see https://github.com/json5/json5/issues/240
const JSON5 = JSON5Module.default || JSON5Module;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Which object is the actual parser, JSON5Module.default or JSON5Module ? Just wondering if you know something about JSON5Module that in some cases would have a default element and in other cases be the parser itself.

update: never mind - I read through json5/json5#240 and can see it may differ based on different loaders.

@lorenwest
Copy link
Collaborator

This looks good - I guess webpack can't know to bundle if modules are dynamically loaded. Since we've made the jump to require json5 this looks like the right way to load it.

@lorenwest
Copy link
Collaborator

Merging @cbazureau webpack fix

@lorenwest lorenwest merged commit aac0693 into node-config:master Jan 18, 2024
@AshotN
Copy link

AshotN commented Jan 31, 2024

Any ETA when this will be included in the next release?

@lorenwest
Copy link
Collaborator

Thank you for the reminder. A new branch and release has been created, and published to NPM. Thank you for your contribution!

@AshotN
Copy link

AshotN commented Feb 1, 2024

I see the new release but it's not in NPM yet

https://www.npmjs.com/package/config?activeTab=versions

@lorenwest
Copy link
Collaborator

Thank you for letting me know. I thought it was published, but the NPM multi-factor auth fooled me.

It's published now.

renovate bot added a commit to tomacheese/cmcutter that referenced this pull request Feb 5, 2024
@renovate
chore(deps): update dependency config to v3.3.11 (#693)
128f60f 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [config](http://github.com/node-config/node-config.git)
([source](https://togithub.com/node-config/node-config)) | [`3.3.10` ->
`3.3.11`](https://renovatebot.com/diffs/npm/config/3.3.10/3.3.11) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/config/3.3.11?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/config/3.3.11?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/config/3.3.10/3.3.11?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/config/3.3.10/3.3.11?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>node-config/node-config (config)</summary>

###
[`v3.3.11`](https://togithub.com/node-config/node-config/releases/tag/v3.3.11)

[Compare
Source](https://togithub.com/node-config/node-config/compare/v3.3.10...v3.3.11)

#### What's Changed

- fix: webpack bundling compatibility by
[@&#8203;cbazureau](https://togithub.com/cbazureau) in
[node-config/node-config#757

#### New Contributors

- [@&#8203;cbazureau](https://togithub.com/cbazureau) made their first
contribution in
[node-config/node-config#757

**Full Changelog**:
node-config/node-config@v3.3.10...v3.3.11

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/tomacheese/cmcutter).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xNzAuMCIsInVwZGF0ZWRJblZlciI6IjM3LjE3MC4wIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIn0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lorenwest lorenwest lorenwest 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.

v3.3.10 is a breaking change for webpack, which does not install json5 dep
3 participants
@cbazureau @lorenwest @AshotN