-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Replace openConfig with resolveConfig #7682
Conversation
|
@@ -96,7 +96,7 @@ describe('MDX plugins', () => { | |||
it('ignores string-based plugins in markdown config', async () => { | |||
const fixture = await buildFixture({ | |||
markdown: { | |||
remarkPlugins: [['remark-toc']], | |||
remarkPlugins: [['remark-toc', {}]], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tuple needs two values or it won't pass config validation. This should be what users have to do right now too it seems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a change in the public APIs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it shouldn't be. Currently if I use the same configuration in the basics
example, I get
10:06:40 PM [astro] Unable to load /Users/bjorn/Work/oss/astro/examples/basics/astro.config.mjs
[config] Astro found issue(s) with your configuration:
! markdown.remarkPlugins.0 Invalid input.
It appears now because the tests will run the zod schema validation for each fixture options.
@@ -23,7 +23,7 @@ describe('getStaticPaths', () => { | |||
const $ = cheerio.load(html); | |||
expect($('p').text()).to.equal('First mdx file'); | |||
expect($('#one').text()).to.equal('hello', 'Frontmatter included'); | |||
expect($('#url').text()).to.equal('/src/content/1.mdx', 'url is included'); | |||
expect($('#url').text()).to.equal('src/content/1.mdx', 'url is included'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The base
handling in the tests previously patches the config, causing the leading slash. In a normal Astro app, there shouldn't be a leading slash.
@@ -96,7 +96,7 @@ describe('MDX plugins', () => { | |||
it('ignores string-based plugins in markdown config', async () => { | |||
const fixture = await buildFixture({ | |||
markdown: { | |||
remarkPlugins: [['remark-toc']], | |||
remarkPlugins: [['remark-toc', {}]], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a change in the public APIs?
Changes
This PR merges to the js-api branch
openConfig
withresolveConfig
(new API)NOTE: There's one more place using
openConfig
, that isdev/restart.ts
. I'd like refactor how it passes around configs during restarts in general, so holding that change for the next PR. After that, we can removeopenConfig
.Testing
Existing tests should pass.
Docs
n/a. internal change.