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

Refactor: move prism to @astrojs/prism/components #2878

Merged
merged 6 commits into from
Mar 24, 2022

Conversation

bholmesdev
Copy link
Contributor

@bholmesdev bholmesdev commented Mar 24, 2022

Changes

  • Adds the <Prism/> component as default export from @astrojs/prism/component
  • Removes the <Prism/> component from astro/components

☝️ Open to avoiding removal if we prefer. I think adding this component to @astrojs/prism could aid in the 1.0 beta move though.

Testing

N/A

Docs

withastro/docs#256

@changeset-bot
Copy link

changeset-bot bot commented Mar 24, 2022

🦋 Changeset detected

Latest commit: 9fa7127

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
astro Patch
@astrojs/prism Patch
@astrojs/markdown-remark Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: astro Related to the core `astro` package (scope) pkg: example Related to an example package (scope) labels Mar 24, 2022
@bholmesdev bholmesdev force-pushed the refactor/move-prism-to-astro-prism-pkg branch from 0f0b14d to e63a0e6 Compare March 24, 2022 20:09
@github-actions github-actions bot removed the pkg: example Related to an example package (scope) label Mar 24, 2022
Copy link
Contributor

@matthewp matthewp left a comment

Choose a reason for hiding this comment

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

Added some comments

"exports": {
".": "./index.mjs"
".": "./index.mjs",
"./components": "./components/index.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, but just @astrojs/prism/component with no (s)? There's not going to be multiple of these are there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess not! I really wanted to export from index.mjs but I can't figure out putting .astro files and ESM exports in the same file... Let me know if this is possible (without building the .astro file to JS of course)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update: changed to a default export:

import Prism from '@astrojs/prism/component';

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah i was thinking that from the main would be nice but can't really do that, too bad. This works fine though, thanks!

@bholmesdev bholmesdev merged commit 2db97f1 into main Mar 24, 2022
@bholmesdev bholmesdev deleted the refactor/move-prism-to-astro-prism-pkg branch March 24, 2022 21:48
@github-actions github-actions bot mentioned this pull request Mar 24, 2022
@FredKSchott
Copy link
Member

This is more than enough for now, but whenever we go to expose this to users I think the /component entrypoint API is a bit unexpected. I'd expect import {Prism} from '@astro/prism' to work.

I know this package is already doing double-duty, But, if the addAstro export will always be the "only used internally" export and Prism is the publicly available one, then I think a switch of the two would make sense when we go to expose this for users.

import {Prism} from '@astrojs/prism';
// in Astro: import {addAstro} from '@astrojs/prism/internal';

@github-actions github-actions bot mentioned this pull request Mar 25, 2022
SiriousHunter pushed a commit to SiriousHunter/astro that referenced this pull request Feb 3, 2023
* feat: add `<Prism/>` to @astrojs/prism/components

* feat: remove `<Prism/>` from astro/components

* refactor: point to index.mjs in import

* refactor: change exp to @astrojs/prism/component

* refactor: remove unecessary index.js

* chore: changeset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants