Add inferring of `development` options from webpack loader mode #2201

remcohaszing · 2022-12-23T14:09:20Z

vercel · 2022-12-23T14:09:24Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
mdx	✅ Ready (Inspect)	Visit Preview	Dec 26, 2022 at 3:03PM (UTC)

codecov-commenter · 2022-12-23T14:14:22Z

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (e39eda8) compared to base (5f68330).
Patch has no changes to coverable lines.

Additional details and impacted files

@@            Coverage Diff            @@
##              main     #2201   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        22           
  Lines         2064      2064           
=========================================
  Hits          2064      2064

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

ChristianMurphy

Loader change looks good 👍
Have a minor question on the test case.

ChristianMurphy · 2022-12-23T14:13:31Z

packages/loader/test/index.test.js

@@ -138,6 +138,12 @@ webpack.mdx:1:22: Unexpected end of file in expression, expected a corresponding
    'should compile (preact)'
  )

+  assert.ok(
+    // @ts-expect-error Type definitions don’t define __source.
+    modPreact.default.default({}).__source,


I'm not sure I follow this test case.
Does this property only exist in development mode?
If so, would we need a test to show it is missing in production mode?

Yes, this property only exists in development mode. Currently a webpack build with mode 'none' is created for React, and one with mode 'development' for Preact. Preact attaches the __source property to the node, React does not (not even in development). I guess React tracks this somewhere internally.

Interesting 🤔
Would it make sense to also have a test in production mode, and verify the property is missing? Or is that assumed/implied by a test elsewhere?

It’s also possible to read the compiled output and look for certain patterns. It’s not very elegant IMO, but it could work.

I think there should be an explanation next to this test: I agree with Christian that reading this test code is confusing.

The reactBuild has mode: 'none', so there it should use a “production” automatic jsx import, and here we should have a development import, perhaps we can t.match for those?

I changed it to use assert.match now. It’s a bit annoying to find a propert string to match, as output changes significantly with the mode, but I found an ok regex to test against.

wooorm · 2022-12-26T07:05:17Z

packages/loader/test/index.test.js

@@ -138,6 +138,12 @@ webpack.mdx:1:22: Unexpected end of file in expression, expected a corresponding
    'should compile (preact)'
  )

+  assert.ok(
+    // @ts-expect-error Type definitions don’t define __source.
+    modPreact.default.default({}).__source,


I think there should be an explanation next to this test: I agree with Christian that reading this test code is confusing.

The reactBuild has mode: 'none', so there it should use a “production” automatic jsx import, and here we should have a development import, perhaps we can t.match for those?

packages/loader/lib/index.js

wooorm · 2022-12-26T17:28:53Z

Thanks Remco!

wooorm · 2023-02-09T19:09:56Z

Thanks, released in 2.3.0

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@mdx-js/react](https://mdxjs.com) ([source](https://togithub.com/mdx-js/mdx)) | [`2.2.1` -> `2.3.0`](https://renovatebot.com/diffs/npm/@mdx-js%2freact/2.2.1/2.3.0) | [![age](https://badges.renovateapi.com/packages/npm/@mdx-js%2freact/2.3.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/@mdx-js%2freact/2.3.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/@mdx-js%2freact/2.3.0/compatibility-slim/2.2.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/@mdx-js%2freact/2.3.0/confidence-slim/2.2.1)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>mdx-js/mdx</summary> ### [`v2.3.0`](https://togithub.com/mdx-js/mdx/releases/tag/2.3.0) [Compare Source](https://togithub.com/mdx-js/mdx/compare/2.2.1...2.3.0) ##### Add - [`12e63e8`](https://togithub.com/mdx-js/mdx/commit/12e63e83) Add improved support for non-React frameworks by [@wooorm](https://togithub.com/wooorm) in [mdx-js/mdx#2255 ##### Fix - [`764d3a2`](https://togithub.com/mdx-js/mdx/commit/764d3a23) Remove whitespace between unraveled text nodes by [@wooorm](https://togithub.com/wooorm) in [mdx-js/mdx#2252 - [`3911148`](https://togithub.com/mdx-js/mdx/commit/39111484) Add `module: node16` to `tsconfig.json` by [@ChristianMurphy](https://togithub.com/ChristianMurphy) in [mdx-js/mdx#2228 - [`600b12a`](https://togithub.com/mdx-js/mdx/commit/600b12ad) Add inferring of `development` options from webpack loader mode by [@remcohaszing](https://togithub.com/remcohaszing) in [mdx-js/mdx#2201 ##### Community - [`f4d78be`](https://togithub.com/mdx-js/mdx/commit/f4d78be5) Add `recma-mdx-displayname` to list of plugins by [@domdomegg](https://togithub.com/domdomegg) in [mdx-js/mdx#2230 - [`1f551a4`](https://togithub.com/mdx-js/mdx/commit/1f551a4d) Add `remark-mdx-chartjs` to list of plugins by [@pangelani](https://togithub.com/pangelani) in [mdx-js/mdx#2221 ##### Misc - [`97b9d59`](https://togithub.com/mdx-js/mdx/commit/97b9d590) Refactor to improve docs by [@wooorm](https://togithub.com/wooorm) in [mdx-js/mdx#2254 - [`8f85b30`](https://togithub.com/mdx-js/mdx/commit/8f85b300) Refactor `tsconfig.json` by [@wooorm](https://togithub.com/wooorm) in [mdx-js/mdx#2250 - [`a17ac87`](https://togithub.com/mdx-js/mdx/commit/a17ac87c) Refactor wording by [@beeburrt](https://togithub.com/beeburrt) in [mdx-js/mdx#2214 **Full Changelog**: mdx-js/mdx@2.2.1...2.3.0 </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. --- - [ ] 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://app.renovatebot.com/dashboard#github/ziyadedher/ziyadedher).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@mdx-js/react](https://mdxjs.com) ([source](https://togithub.com/mdx-js/mdx)) | [`2.2.1` -> `2.3.0`](https://renovatebot.com/diffs/npm/@mdx-js%2freact/2.2.1/2.3.0) | [![age](https://badges.renovateapi.com/packages/npm/@mdx-js%2freact/2.3.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/@mdx-js%2freact/2.3.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/@mdx-js%2freact/2.3.0/compatibility-slim/2.2.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/@mdx-js%2freact/2.3.0/confidence-slim/2.2.1)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>mdx-js/mdx</summary> ### [`v2.3.0`](https://togithub.com/mdx-js/mdx/releases/tag/2.3.0) [Compare Source](https://togithub.com/mdx-js/mdx/compare/2.2.1...2.3.0) ##### Add - [`12e63e8`](https://togithub.com/mdx-js/mdx/commit/12e63e83) Add improved support for non-React frameworks by [@wooorm](https://togithub.com/wooorm) in [mdx-js/mdx#2255 ##### Fix - [`764d3a2`](https://togithub.com/mdx-js/mdx/commit/764d3a23) Remove whitespace between unraveled text nodes by [@wooorm](https://togithub.com/wooorm) in [mdx-js/mdx#2252 - [`3911148`](https://togithub.com/mdx-js/mdx/commit/39111484) Add `module: node16` to `tsconfig.json` by [@ChristianMurphy](https://togithub.com/ChristianMurphy) in [mdx-js/mdx#2228 - [`600b12a`](https://togithub.com/mdx-js/mdx/commit/600b12ad) Add inferring of `development` options from webpack loader mode by [@remcohaszing](https://togithub.com/remcohaszing) in [mdx-js/mdx#2201 ##### Community - [`f4d78be`](https://togithub.com/mdx-js/mdx/commit/f4d78be5) Add `recma-mdx-displayname` to list of plugins by [@domdomegg](https://togithub.com/domdomegg) in [mdx-js/mdx#2230 - [`1f551a4`](https://togithub.com/mdx-js/mdx/commit/1f551a4d) Add `remark-mdx-chartjs` to list of plugins by [@pangelani](https://togithub.com/pangelani) in [mdx-js/mdx#2221 ##### Misc - [`97b9d59`](https://togithub.com/mdx-js/mdx/commit/97b9d590) Refactor to improve docs by [@wooorm](https://togithub.com/wooorm) in [mdx-js/mdx#2254 - [`8f85b30`](https://togithub.com/mdx-js/mdx/commit/8f85b300) Refactor `tsconfig.json` by [@wooorm](https://togithub.com/wooorm) in [mdx-js/mdx#2250 - [`a17ac87`](https://togithub.com/mdx-js/mdx/commit/a17ac87c) Refactor wording by [@beeburrt](https://togithub.com/beeburrt) in [mdx-js/mdx#2214 **Full Changelog**: mdx-js/mdx@2.2.1...2.3.0 </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. --- - [ ] 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://app.renovatebot.com/dashboard#github/ziyadedher/ziyadedher).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Infer development option from Webpack mode

ea98100

Closes #2198

remcohaszing added 🦋 type/enhancement This is great to have 🗄 area/interface This affects the public interface labels Dec 23, 2022

remcohaszing requested a review from a team December 23, 2022 14:09

vercel bot deployed to Preview December 23, 2022 14:11 View deployment

ChristianMurphy approved these changes Dec 23, 2022

View reviewed changes

wooorm reviewed Dec 26, 2022

View reviewed changes

Use output match to detect dev mode in webpack bundle

e39eda8

vercel bot deployed to Preview December 26, 2022 15:03 View deployment

wooorm approved these changes Dec 26, 2022

View reviewed changes

wooorm changed the title ~~Infer development option from Webpack mode~~ Add inferring of development options from webpack loader mode Dec 26, 2022

wooorm merged commit 600b12a into mdx-js:main Dec 26, 2022

wooorm added the 💪 phase/solved Post is done label Dec 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add inferring of `development` options from webpack loader mode #2201

Add inferring of `development` options from webpack loader mode #2201

remcohaszing commented Dec 23, 2022

vercel bot commented Dec 23, 2022 •

edited

codecov-commenter commented Dec 23, 2022 •

edited

ChristianMurphy left a comment

ChristianMurphy Dec 23, 2022

remcohaszing Dec 23, 2022

ChristianMurphy Dec 23, 2022

remcohaszing Dec 23, 2022

wooorm Dec 26, 2022

remcohaszing Dec 26, 2022

wooorm Dec 26, 2022

wooorm commented Dec 26, 2022

wooorm commented Feb 9, 2023

Add inferring of development options from webpack loader mode #2201

Add inferring of development options from webpack loader mode #2201

Conversation

remcohaszing commented Dec 23, 2022

vercel bot commented Dec 23, 2022 • edited

codecov-commenter commented Dec 23, 2022 • edited

Codecov Report

ChristianMurphy left a comment

Choose a reason for hiding this comment

ChristianMurphy Dec 23, 2022

Choose a reason for hiding this comment

remcohaszing Dec 23, 2022

Choose a reason for hiding this comment

ChristianMurphy Dec 23, 2022

Choose a reason for hiding this comment

remcohaszing Dec 23, 2022

Choose a reason for hiding this comment

wooorm Dec 26, 2022

Choose a reason for hiding this comment

remcohaszing Dec 26, 2022

Choose a reason for hiding this comment

wooorm Dec 26, 2022

Choose a reason for hiding this comment

wooorm commented Dec 26, 2022

wooorm commented Feb 9, 2023

Add inferring of `development` options from webpack loader mode #2201

Add inferring of `development` options from webpack loader mode #2201

vercel bot commented Dec 23, 2022 •

edited

codecov-commenter commented Dec 23, 2022 •

edited