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.

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

fix(gatsby-plugin-mdx) for image import name collisions in mdx files #35028

Conversation

illogic-al
Copy link

@illogic-al illogic-al commented Mar 2, 2022

This commit allows importing files with the same import name across different files.
Tries to be valid for import pic from file.jpeg or import * as pic from file.jpeg imports.

Description

Given two files—file A located at src/subdir1/file.mdx and file B located at src/subdir2/file.mdx— any imports with the same import name in both files will be 'compressed' to just one import: the last one.

The changes introduced here make image imports across mdx files scoped to those specific files.

Documentation

Related Issues

Fixes closed issue #17119. Gives more runway for (in progress?) refactor of #25069 which is listed as target for providing a solution for 17119.

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Mar 2, 2022
@illogic-al illogic-al changed the title Fix import name collisions for images in mdx files by creating namespace fix(gatsby) for image import name collisions in mdx files Mar 2, 2022
@illogic-al illogic-al force-pushed the illogic-al-mdx-image-import-namespacing branch 2 times, most recently from 5ba17f2 to b875961 Compare March 2, 2022 21:54
@illogic-al illogic-al changed the title fix(gatsby) for image import name collisions in mdx files fix(gatsby-plugin-mdx) for image import name collisions in mdx files Mar 2, 2022
@illogic-al illogic-al force-pushed the illogic-al-mdx-image-import-namespacing branch from a2fbcc6 to 7ba5a33 Compare March 2, 2022 22:48
@illogic-al
Copy link
Author

Fix lint errors but tests are still failing (I don't think that's because of any changes I made).
If I should make these changes against another branch (release?) let me know and I can do so.

illogic-al and others added 5 commits March 3, 2022 19:40
This commit allows importing files with the same import name across different files.
Tries to be valid for `import pic from file.jpeg` or `import * as pic from file.jpeg` imports.
@illogic-al illogic-al force-pushed the illogic-al-mdx-image-import-namespacing branch from e6103e6 to 1ac268a Compare March 4, 2022 00:40
…s between identically named images.

Model actual situation being fixed.
@illogic-al
Copy link
Author

The files checked in now models the situation i see with our project. And if you check in the cache directory

caches/gatsby-plugin-mdx/mdx-scopes-dir on  master [📝🤷⇡4] via  v16.11.0
❯ rg import
dd93cb4a21d61061cfe36d3c15e2dc90.js
1:import { Message } from "theme-ui";
2:import * as React from 'react';

91e47e0ef8fd7e9b3681c235252d2f02.js
1:import HMRImportEditComponent from "../../../../src/components/hmr-component-edit";
2:import HMRPropEditComponent from "../../../../src/components/hmr-prop-edit";
3:import * as React from 'react';

17ca682c1f25e1646fd4a5e0b6bc382e.js
1:import prettyImage from "../../../../src/pages/mdx_import2/gatsby-icon.png";
2:import * as React from 'react';

e92f8988d65cf25c087d226e6c0ef06f.js
1:import * as React from 'react';

Note the prettyImage import. Only one exists. mdx_import2 directory, got processed last apparently, and "wins".

caches/gatsby-plugin-mdx/mdx-scopes-dir on  master [📝🤷⇡4] via  v16.11.0
❯ rg import
dd93cb4a21d61061cfe36d3c15e2dc90.js
1:import { Message } from "theme-ui";
2:import * as React from 'react';

bac3691348474d53375f4dc253bb37d1.js
1:import prettyImage_2e51e8ec43b3502fb938c3ae321dc384 from "../../../../src/pages/mdx_import1/gatsby-icon.png";
2:import * as React from 'react';

46ef31c9a33094f0fcb1883ca964d1a6.js
1:import prettyImage_3fd5b8eaaf3957b9848de768224aca5b from "../../../../src/pages/mdx_import2/gatsby-icon.png";
2:import * as React from 'react';

91e47e0ef8fd7e9b3681c235252d2f02.js
1:import HMRImportEditComponent from "../../../../src/components/hmr-component-edit";
2:import HMRPropEditComponent from "../../../../src/components/hmr-prop-edit";
3:import * as React from 'react';

e92f8988d65cf25c087d226e6c0ef06f.js
1:import * as React from 'react';

``prettyImage_xxxxxxxx` imports show up twice above, because I manually patched the on-create-node.js file in the test directories node modules.

But that brings up 2 issues.

  1. These tests are not testing the current version of the mdx plugin. The version defined in the package.json is quite old.
  2. Even after updating to a recent version, my changes won't be reflected without patching because we're not referencing the locally modified version here.

@LekoArts Any way to fix these issues?

@LekoArts LekoArts added topic: remark/mdx Related to Markdown, remark & MDX ecosystem and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Mar 8, 2022
@LekoArts
Copy link
Contributor

LekoArts commented Mar 8, 2022

The CI that runs your tests uses gatsby-dev-cli to copy over the changes you made to the test. You can also locally do this: https://www.gatsbyjs.com/contributing/code-contributions/#testing-out-changes-in-an-example-project

So you'd have in the root of the monorepo running:

yarn watch --scope=gatsby-plugin-mdx

And then inside e2e-tests/mdx you would run:

gatsby-dev --packages gatsby-plugin-mdx

Then your local changes are copied over to the example project.

@LekoArts LekoArts self-assigned this Mar 8, 2022
@illogic-al
Copy link
Author

Any word on this?

jonlong pushed a commit to jonlong/portfolio that referenced this pull request Jun 7, 2022
@LekoArts
Copy link
Contributor

Hey! Sorry for the silence here.

MDX v2 is on the horizon: #25068
We completely focus on MDX v2 at the moment and won't focus on the old version of gatsby-plugin-mdx anymore.

Please try those canaries out and if this still occurs there, comment on the discussion. Thanks!

@LekoArts LekoArts closed this Jul 26, 2022
@illogic-al
Copy link
Author

Ah. That is unfortunate as we don't have any need to move to MDX2 (currently), and the last time I tried I don't believe it was a seamless update (within gatsby). Perhaps we'll be able to try again in the future. Thanks for updating us on the progress though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: remark/mdx Related to Markdown, remark & MDX ecosystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants