Skip to content

Type checking preset options#10927

Closed
JLHwung wants to merge 5 commits intobabel:next-8-devfrom
JLHwung:type-checking-preset-options
Closed

Type checking preset options#10927
JLHwung wants to merge 5 commits intobabel:next-8-devfrom
JLHwung:type-checking-preset-options

Conversation

@JLHwung
Copy link
Copy Markdown
Contributor

@JLHwung JLHwung commented Dec 26, 2019 •
edited by gitpod-io bot
Loading

Q                       A
Fixed Issues? Add strict type checking and did you mean tips to preset-{flow,react,typescript}
Major: Breaking Change? Yes
Minor: New Feature? Yes
Tests Added + Pass? Yes
License MIT

The design is copied from preset-env. To avoid duplicate codes, we can extract the validators into a helper library. Ideally we can use schema to check these types, but this PR should include the only visible breaking changes: we are now throwing type errors like useBuiltIns: 0 or invalid options useBuiltins: true instead of accepting/discarding them silently.

Todo:

  • Add test on invalid top level options.

@JLHwung JLHwung added area: errors PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release labels Dec 26, 2019
@nicolo-ribaudo nicolo-ribaudo mentioned this pull request Dec 26, 2019
Open
34 tasks
@JLHwung JLHwung force-pushed the type-checking-preset-options branch from 560296e to 725c437 Compare January 14, 2020 20:26
@JLHwung JLHwung marked this pull request as ready for review January 14, 2020 20:28
@milesj
Copy link
Copy Markdown

milesj commented Jan 14, 2020

Shameless plug. I wrote a library to do just this with a clean fluent API. https://github.com/milesj/optimal

@JLHwung
Copy link
Copy Markdown
Contributor Author

JLHwung commented Jan 14, 2020

@milesj Thanks for the info.

We haven't decided yet how the preset option validation should be reworked. During talks to @nicolo-ribaudo, he proposes a JSON-schema based validator so we can ship schema for better integration to IDE. The goal is to help developers write correct configuration instead of to discourage them via complicated validate message.

This PR is only meant to extend the current validation logic from preset-env to other three babel presets. We can switch to a more formal validation libraries later but this PR should be complete for UI visibility.

@JLHwung JLHwung force-pushed the type-checking-preset-options branch 2 times, most recently from 07f12f7 to cb15988 Compare February 29, 2020 18:41
@JLHwung JLHwung changed the base branch from master to next-8-dev February 29, 2020 18:41
@JLHwung JLHwung mentioned this pull request Feb 29, 2020
Merged
@JLHwung JLHwung force-pushed the type-checking-preset-options branch from b193be2 to 43418f1 Compare June 24, 2020 21:27
@JLHwung
Copy link
Copy Markdown
Contributor Author

JLHwung commented Jun 24, 2020

This PR has been revamped featuring less boilerplate of option validation.

@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented Jun 24, 2020 •
edited
Loading

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/24696/

@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci bot commented Jun 24, 2020 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 6cc4a72:

Sandbox Source
spring-sound-ocqzt Configuration
priceless-golick-9tk27 Configuration

@JLHwung JLHwung added this to the Babel 8.0 milestone Jun 24, 2020
kaicataldo
kaicataldo reviewed Jun 25, 2020
View reviewed changes
@JLHwung JLHwung force-pushed the type-checking-preset-options branch from 46678cc to 007c944 Compare June 26, 2020 14:09
@JLHwung JLHwung force-pushed the type-checking-preset-options branch from 007c944 to 6cc4a72 Compare June 26, 2020 14:19
@JLHwung JLHwung mentioned this pull request Jun 26, 2020
Merged
1 task
@JLHwung JLHwung mentioned this pull request Aug 25, 2020
Merged
@JLHwung JLHwung mentioned this pull request Nov 11, 2020
Merged
@JLHwung JLHwung mentioned this pull request Dec 8, 2020
Merged
This was referenced Feb 2, 2021
Merged
Merged
@JLHwung JLHwung removed this from the Babel 8.0 milestone Feb 3, 2021
@JLHwung
Copy link
Copy Markdown
Contributor Author

JLHwung commented Aug 9, 2023

This PR has been replaced by #12460, #12741 and #12751.

@JLHwung JLHwung closed this Aug 9, 2023
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Nov 9, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@kaicataldo kaicataldo kaicataldo left review comments

@nicolo-ribaudo nicolo-ribaudo Awaiting requested review from nicolo-ribaudo

Assignees

No one assigned

Labels

area: errors outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release PR: Ready For Review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@JLHwung @milesj @babel-bot @kaicataldo