Skip to content

Conversation

@evwilkin
Copy link
Member

Closes #3525

This PR updates the conversion of relative import paths to absolute import paths, to be used in Codesandbox where the relative files aren't present.

Previously:

  • mdx-hast-to-jsx would identify any relative imports in example code (import Something from '../Something.js'), parse out the name of the imported item, and build a map matching this relative imported item to the absolute import path for it
    • { Something: '@patternfly/some-package/Something.js' }
    • This is referenced in codesandbox.js to use simple string replacement to swap out the relative path with the absolute path in the example code before sending to Codesandbox example
    • Code had bugs and added extra complexity by parsing out imported item names from relative import path, when we already have relative & absolute paths which is all we really need

Now:

  • mdx-hast-to-jsx maps the relative import path (not the name of the imported item) directly to the absolute import path
    • { '../../Something.js': '@patternfly/some-package/Something.js' }
    • This also resolve the issue of destructuring relative imports including multiple items (ex: import { rows, columns } from '../../Data.js') - what is being imported doesn't really matter, we're just concerned with finding the relative file where it's coming from and getting the absolute path to that file.

@patternfly-build
Copy link
Collaborator

patternfly-build commented May 17, 2023

Copy link
Contributor

@jenny-s51 jenny-s51 left a comment

Choose a reason for hiding this comment

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

This is looking awesome @evwilkin!

Looked through a bunch of codesandbox demos that use relative import paths and lots of them have now been resolved with this update... Some icons within certain demos are still not rendering due to some residual image paths that should updated in react; opened patternfly/patternfly-react#9140 to track these and assigned myself to these cleanup tasks

LGTM 👍

@evwilkin evwilkin requested a review from kmcfaul May 17, 2023 19:52
jenny-s51
jenny-s51 previously approved these changes Jun 23, 2023
@evwilkin evwilkin force-pushed the fix/3525-table-relative-imports branch from 2a84dc0 to d7789ae Compare July 13, 2023 21:30
@jenny-s51 jenny-s51 requested a review from nicolethoen August 8, 2023 20:58
@nicolethoen nicolethoen merged commit f30013e into patternfly:main Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug - Card view and Primary detail - card view demos don't open in CodeSandbox Bug - full screen table demos don't open in codesandbox

4 participants