-
Notifications
You must be signed in to change notification settings - Fork 249
Breaking: v6 with backward compatibility #3370
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
base: master
Are you sure you want to change the base?
Conversation
| const isProduction = false; | ||
| // These will be overridden/added to in the package.json of any plugin | ||
| // they are here for backward compatibility only | ||
| const v5toV6DefaultMappings = { |
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.
with the maps and paths now specified here can we remove the corresponding config from scriptLoader?
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. The ones in the scriptLoader.js are client-side and they happen before adapt.min.js is loaded, the mappings from the compiler happen at the top of adapt.min.js, after those modules are needed. Oftentimes external libraries/ files will expect jQuery at minimum and will need access to jQuery before adapt.min.js is loaded.
We will be able to remove some of them as we move over to node_modules, specifically: regenerator-runtime, core-js, react, react-dom, html-react-parser and semver. The others will be more difficult.
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.
ah, of course, though is the scriptLoader map used at all?
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 only use I saw was spoor's offline_API_wrapper.js, but I think that has recently been moved into compilation, right?
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 map properties are used heavilly in adapt-devtools. The paths section is used further down the scriptLoader.js to load the foundational libraries+templates and some of those libraries are used by name at various places throughout the core and plugins.
map
coreJS: 'core/js',
coreViews: 'core/js/views',
coreModels: 'core/js/models',
coreCollections: 'core/js/collections'
These shortcuts were mostly removed when we switched to babel and jsdoc, both of which really only work correctly with resolvable file paths.
#930
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.
Effectively, when core is updated to switch to node_modules, we can also drop the scriptLoader.js directives, where reasonable to do so, considering backward compatibility and load order. jQuery, jqueryMobile, underscore, backbone, handlebars and Modernizer will be most difficult I imagine. imageReady, inview and jquery.resize are libraries I made, so they should be quite easy to integrate into the core. bowser, scrollTo, requirejs and velocity I haven't analysed yet.
fixes #2526
fixes #2363
original issues #3044
original prototype #3324
What's changed?
src/coursein favour ofbuild/courseTesting
ctrl+c, then runTodo
adapt installcommand etc will be made v6 compatible Add npm layer adapt-cli#175includesdo not yet work with dependent extensions, such that if resources is included but drawer is not, drawer will not yet be added to the construction phase