Skip to content

Conversation

@Daniel-Knights
Copy link
Contributor

This PR adds the tsconfig.core.json changes from https://github.com/IQGeo/myworld-product-core/pull/2060.

The myWorld-client/react import alias is comprised of two separately exported modules from core: core/client/hooks and core/client/uiComponents/react. compilerOptions.paths doesn't support merging two separate modules, so an ambient module declaration was added instead. This declaration references @internal/hooks and @internal/uiComponents/react, both of which need to be aliased in the consuming project.

@Daniel-Knights
Copy link
Contributor Author

Is the idea with the dev branch that it gets merged into main when a new version of Platform is released? The changes in this PR should only be merged once 7.4 is released.

@luiscamachopt
Copy link
Contributor

Is the idea with the dev branch that it gets merged into main when a new version of Platform is released? The changes in this PR should only be merged once 7.4 is released.

No, we can have several releases of the template before 7.4 is released.

Can we keep the line being removed to keep compatibility with pre 7.4 versions? Expectation is that people should be able to continue using (and pull&merge) the template with 7.2 or 7.3 even after 7.4 is released...

Copy link
Contributor

@luiscamachopt luiscamachopt left a comment

Choose a reason for hiding this comment

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

I think we need to discuss options for this better

Comment on lines +17 to +18
"@internal/hooks": ["../core/client/hooks"],
"@internal/uiComponents/react": ["../core/client/uiComponents/react"]
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks quite awkward, sounds like we should reorganise the code in core for 7.4 to a more standard pattern(keeping existing imports working) and get more straightforward configuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A single entry point to match the single import path would make sense, I think. Maybe a myWorld-react.js like we have myWorld-client.js?

Copy link
Contributor

Choose a reason for hiding this comment

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

In 7.4, I think we need to keep myWorld-client.js for 7.4. I am curious about the change added to 7.4 that would require this change.

@Daniel-Knights
Copy link
Contributor Author

Is the idea with the dev branch that it gets merged into main when a new version of Platform is released? The changes in this PR should only be merged once 7.4 is released.

No, we can have several releases of the template before 7.4 is released.

Can we keep the line being removed to keep compatibility with pre 7.4 versions? Expectation is that people should be able to continue using (and pull&merge) the template with 7.2 or 7.3 even after 7.4 is released...

We can have both.

I'll add a comment to tsconfig.core.json to mention that backwards compatibility should be a consideration when making changes

Copy link
Contributor

@mstrong98 mstrong98 left a comment

Choose a reason for hiding this comment

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

Thanks Dan, I think we need to revisit the changes made in 7.4 for this.

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.

4 participants