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

chore: remove dependency on tv4-formats #498

Closed
wants to merge 1 commit into from

Conversation

sarfata
Copy link
Contributor

@sarfata sarfata commented Aug 15, 2018

TV4-Format provides some JSON validation helper for common string
formats. It was loaded only when validating the full model (not delta)
and all the tests run without it so I propose we remove it.

Reason for me looking into this is that this library causes problem with
webpack because of a heuristic they use to require one of their
dependency. This is a known issue that they have fixed in a later
version and I have confirmed that upgrading the package fixes the issue
for us too. However, if we do not use the library I think it's better to
completely remove it.

For more info: ikr/tv4-formats@61450dc

TV4-Format provides some JSON validation helper for common string
formats. It was loaded only when validating the full model (not delta)
and all the tests run without it so I propose we remove it.

Reason for me looking into this is that this library causes problem with
webpack because of a heuristic they use to require one of their
dependency. This is a known issue that they have fixed in a later
version and I have confirmed that upgrading the package fixes the issue
for us too. However, if we do not use the library I think it's better to
completely remove it.

For more info: ikr/tv4-formats@61450dc
@sarfata
Copy link
Contributor Author

sarfata commented Aug 31, 2018

ping? @tkurki are you ok merging this?

@tkurki
Copy link
Member

tkurki commented Sep 1, 2018

all the tests run without it does not mean that it is not used: try mangling a timestamp in one of the test full files and run the tests - they will fail. This dependency adds validation of date-time JSON Schema data type that is in use in the schema.

Options on how to move forward:

Then there is the issue of separating the schema js API #435 (including validation) from the actual schema data itself, versioning them separately. Then we would have the option two npm packages, one with schema data and js access to it and another for validation, versioned independently.

@sarfata
Copy link
Contributor Author

sarfata commented Sep 1, 2018

@tjkurki Sorry about that. My bad.

Pushed #501 instead. Hopefully this works for you.

@sarfata sarfata closed this Sep 1, 2018
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.

2 participants